Skip to content

Removing Blocking Wait for Close in RecoverySourceHandler#86127

Merged
original-brownbear merged 2 commits intoelastic:masterfrom
original-brownbear:85839
May 17, 2022
Merged

Removing Blocking Wait for Close in RecoverySourceHandler#86127
original-brownbear merged 2 commits intoelastic:masterfrom
original-brownbear:85839

Conversation

@original-brownbear
Copy link
Copy Markdown
Contributor

Remove the blocking wait when releasing safe commits or store references on the recovery source.
This seems safe enough these days with #85238 not tripping
and the assertion that makes sure we never submit the close task to an already shut-down pool

Added the assertion to ensure no exception on close here since we already assert that the store closes without exception and neither should closing the commit ref.

closes #85839

Remove the blocking wait when releasing safe commits or store references on the recovery source.
This seems safe enough these days with #85238 not tripping
and the assertion that makes sure we never submit the close task to an already shut-down pool

closes #85839
@original-brownbear original-brownbear added >bug :Distributed/Engine Anything around managing Lucene and the Translog in an open shard. v8.3.0 labels Apr 25, 2022
@elasticmachine elasticmachine added the Team:Distributed Meta label for distributed team. label Apr 25, 2022
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Hi @original-brownbear, I've created a changelog YAML for you.

@original-brownbear
Copy link
Copy Markdown
Contributor Author

@DaveCTurner ++ to your comment in the original issue, I think fire and forget is fine here now. When I originally added the todo (and before that when this code was added) I believe we simply weren't in as clean a state as far as waiting for the recovery tasks up the stack on node shutdown and also didn't have the assertion that closing the store doesn't throw yet which might fire-and-forget not so great back in the day.

Copy link
Copy Markdown
Member

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

LGTM

@original-brownbear
Copy link
Copy Markdown
Contributor Author

Thanks David!

@original-brownbear original-brownbear merged commit 8ff0189 into elastic:master May 17, 2022
@original-brownbear original-brownbear deleted the 85839 branch May 17, 2022 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>bug :Distributed/Engine Anything around managing Lucene and the Translog in an open shard. Team:Distributed Meta label for distributed team. v8.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RecoverySourceHandler#runWithGenericThreadPool caused deadlock

4 participants