Skip to content

Removing Blocking Wait for Close in RecoverySourceHandler #86127

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 17, 2022
Merged

Removing Blocking Wait for Close in RecoverySourceHandler #86127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
33fb8f5
Removing Blocking Wait for Close in RecoverySourceHandler
original-brownbear Apr 25, 2022
738d85c
Update docs/changelog/86127.yaml
original-brownbear Apr 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/changelog/86127.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 86127
summary: Removing Blocking Wait for Close in `RecoverySourceHandler`
area: Engine
type: bug
issues:
- 85839
23 changes: 11 additions & 12 deletions server/src/main/java/org/elasticsearch/indices/recovery/RecoverySourceHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@
import org.elasticsearch.ExceptionsHelper;
import org.elasticsearch.Version;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.ActionRunnable;
import org.elasticsearch.action.StepListener;
import org.elasticsearch.action.support.PlainActionFuture;
import org.elasticsearch.action.support.ThreadedActionListener;
import org.elasticsearch.action.support.replication.ReplicationResponse;
import org.elasticsearch.cluster.routing.IndexShardRoutingTable;
Expand All @@ -39,7 +37,6 @@
import org.elasticsearch.common.util.concurrent.CountDown;
import org.elasticsearch.common.util.concurrent.FutureUtils;
import org.elasticsearch.common.util.concurrent.ListenableFuture;
import org.elasticsearch.core.CheckedRunnable;
import org.elasticsearch.core.IOUtils;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.core.Releasable;
Expand Down Expand Up @@ -458,7 +455,7 @@ public void onFailure(Exception e) {
*/
private Releasable acquireStore(Store store) {
store.incRef();
return Releasables.releaseOnce(() -> runWithGenericThreadPool(store::decRef));
return Releasables.releaseOnce(() -> closeOnGenericThreadPool(store::decRef));
}

/**
Expand All @@ -468,17 +465,19 @@ private Releasable acquireStore(Store store) {
*/
private Engine.IndexCommitRef acquireSafeCommit(IndexShard shard) {
final Engine.IndexCommitRef commitRef = shard.acquireSafeIndexCommit();
return new Engine.IndexCommitRef(commitRef.getIndexCommit(), () -> runWithGenericThreadPool(commitRef::close));
return new Engine.IndexCommitRef(commitRef.getIndexCommit(), () -> closeOnGenericThreadPool(commitRef));
}

private void runWithGenericThreadPool(CheckedRunnable<Exception> task) {
final PlainActionFuture<Void> future = new PlainActionFuture<>();
private void closeOnGenericThreadPool(Closeable closeable) {
assert threadPool.generic().isShutdown() == false;
// TODO: We shouldn't use the generic thread pool here as we already execute this from the generic pool.
// While practically unlikely at a min pool size of 128 we could technically block the whole pool by waiting on futures
// below and thus make it impossible for the store release to execute which in turn would block the futures forever
threadPool.generic().execute(ActionRunnable.run(future, task));
FutureUtils.get(future);
threadPool.generic().execute(() -> {
try {
closeable.close();
} catch (Exception e) {
assert false : e;
logger.warn(new ParameterizedMessage("Exception while closing [{}]", closeable), e);
}
});
}

static final class SendFileResult {
Expand Down