Fix Get Snapshots Request Cancellation with ignore_unavailable=true (#…

…78004) (#78057) Short-circuit the failure method when cancelled just like in the fail fast case. Also, remove the special case handling that asserts but swallows exceptions in production for when ignoring unavailable to not swallow the task cancellation exception. closes #77980
elastic · Sep 21, 2021 · 4bb0615 · 4bb0615
1 parent 9fb2eb1
commit 4bb0615
Show file tree

Hide file tree

Showing 3 changed files with 5 additions and 17 deletions.
diff --git a/...t-http/src/test/java/org/elasticsearch/http/snapshots/RestGetSnapshotsCancellationIT.java b/...t-http/src/test/java/org/elasticsearch/http/snapshots/RestGetSnapshotsCancellationIT.java
@@ -41,6 +41,9 @@ public void testGetSnapshotsCancellation() throws Exception {
         repository.setBlockOnAnyFiles();
 
         final Request request = new Request(HttpGet.METHOD_NAME, "/_snapshot/" + repoName + "/*");
+        if (randomBoolean()) {
+            request.addParameter("ignore_unavailable", "true");
+        }
         final PlainActionFuture<Response> future = new PlainActionFuture<>();
         final Cancellable cancellable = getRestClient().performRequestAsync(request, wrapAsRestResponseListener(future));
 

diff --git a/...ava/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java b/...ava/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java
@@ -8,8 +8,6 @@
 
 package org.elasticsearch.action.admin.cluster.snapshots.get;
 
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
 import org.apache.lucene.util.CollectionUtil;
 import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.action.ActionListener;
@@ -66,8 +64,6 @@
  */
 public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSnapshotsRequest, GetSnapshotsResponse> {
 
-    private static final Logger logger = LogManager.getLogger(TransportGetSnapshotsAction.class);
-
     private final RepositoriesService repositoriesService;
 
     @Inject
@@ -417,18 +413,7 @@ private void snapshots(
                 ignoreUnavailable == false,
                 task::isCancelled,
                 (context, snapshotInfo) -> snapshotInfos.add(snapshotInfo),
-                ignoreUnavailable ? ActionListener.runAfter(new ActionListener<Void>() {
-                    @Override
-                    public void onResponse(Void unused) {
-                        logger.trace("done fetching snapshot infos [{}]", snapshotIdsToIterate);
-                    }
-
-                    @Override
-                    public void onFailure(Exception e) {
-                        assert false : new AssertionError("listener should always complete successfully for ignoreUnavailable=true", e);
-                        logger.warn("failed to fetch snapshot info for some snapshots", e);
-                    }
-                }, () -> allDoneListener.onResponse(null)) : allDoneListener
+                allDoneListener
             )
         );
     }

diff --git a/server/src/main/java/org/elasticsearch/repositories/GetSnapshotInfoContext.java b/server/src/main/java/org/elasticsearch/repositories/GetSnapshotInfoContext.java
@@ -125,7 +125,7 @@ public void onResponse(SnapshotInfo snapshotInfo) {
     @Override
     public void onFailure(Exception e) {
         assert Repository.assertSnapshotMetaThread();
-        if (abortOnFailure) {
+        if (abortOnFailure || isCancelled()) {
             if (counter.fastForward()) {
                 failDoneListener(e);
             }