Skip to content

Conversation

@jjezra
Copy link
Contributor

@jjezra jjezra commented Nov 11, 2025

After setting indexing type the future continues to update indexing heartbeats. The thrown exceptions in this path should be replaced with a completeExceptionally to halt this future.
This may fix "runner had been closed" exceptions.

resolves #3741

 After setting indexing type the future continues to update indexing heartbeats. The thrown exceptions in this path should be replaced with a completeExceptionally to halt this future.
 This may fix "runner had been closed" exceptions.

 resolves FoundationDB#3741
@jjezra jjezra added the bug fix Change that fixes a bug label Nov 11, 2025
@jjezra jjezra marked this pull request as ready for review November 11, 2025 22:46
@jjezra jjezra requested a review from ScottDugas November 11, 2025 22:46
.build();
}

private CompletableFuture<Void> failedFutureVoid(Throwable ex) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it would be worthwhile to put this in MoreAsyncUtil and make it generic (CompletableFuture<T>).
There's definitely other code doing this. (honestly I was a bit surprised it wasn't supported in CompletableFuture or already existing in MoreAsyncUtil)

if (isTypeStampBlocked(savedStamp) && !policy.shouldAllowUnblock(savedStamp.getBlockID())) {
// Indexing is blocked
throw newPartlyBuiltException(savedStamp, newStamp, index);
return failedFutureVoid(newPartlyBuiltException(savedStamp, newStamp, index));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I understand how the change here would change any behavior?
I think it's a bit cleaner, but everywhere that you were throwing, you now return, and anything above shouldn't really know whether it completed exceptionally because that was called or an exception was thrown, I think.

@jjezra jjezra marked this pull request as draft November 12, 2025 22:57
@jjezra jjezra closed this Nov 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug fix Change that fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Avoid updating heartbeat if indexing type had failed to be set

2 participants