Skip to content

Conversation

@original-brownbear
Copy link
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 elastic#85238 not tripping
and the assertion that makes sure we never submit the close task to an already shut-down pool

closes elastic#85839
@original-brownbear original-brownbear added >bug :Distributed Indexing/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 (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Apr 25, 2022
@elasticmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

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

@original-brownbear
Copy link
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
Contributor

@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
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 Indexing/Engine Anything around managing Lucene and the Translog in an open shard. Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v8.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RecoverySourceHandler#runWithGenericThreadPool caused deadlock

4 participants