From 33fb8f5b923daac47fbbbcf096e9982907444a9e Mon Sep 17 00:00:00 2001
From: Armin Braun <me@obrown.io>
Date: Mon, 25 Apr 2022 09:48:56 +0200
Subject: [PATCH 1/2] Removing Blocking Wait for Close in RecoverySourceHandler

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

closes #85839
---
 .../recovery/RecoverySourceHandler.java       | 23 +++++++++----------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/server/src/main/java/org/elasticsearch/indices/recovery/RecoverySourceHandler.java b/server/src/main/java/org/elasticsearch/indices/recovery/RecoverySourceHandler.java
index f882dac5779e9..59ebda50b1d55 100644
--- a/server/src/main/java/org/elasticsearch/indices/recovery/RecoverySourceHandler.java
+++ b/server/src/main/java/org/elasticsearch/indices/recovery/RecoverySourceHandler.java
@@ -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;
@@ -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;
@@ -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));
     }
 
     /**
@@ -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 {

From 738d85c19c9290015bf0aeea2bfce6bd1430ffbe Mon Sep 17 00:00:00 2001
From: Armin Braun <me@obrown.io>
Date: Mon, 25 Apr 2022 09:59:08 +0200
Subject: [PATCH 2/2] Update docs/changelog/86127.yaml

---
 docs/changelog/86127.yaml | 6 ++++++
 1 file changed, 6 insertions(+)
 create mode 100644 docs/changelog/86127.yaml

diff --git a/docs/changelog/86127.yaml b/docs/changelog/86127.yaml
new file mode 100644
index 0000000000000..1e108c14bd7f7
--- /dev/null
+++ b/docs/changelog/86127.yaml
@@ -0,0 +1,6 @@
+pr: 86127
+summary: Removing Blocking Wait for Close in `RecoverySourceHandler`
+area: Engine
+type: bug
+issues:
+ - 85839