Fix Ex. Handling in SnapshotsService#snapshots (#47507)

* Fix Ex. Handling in SnapshotsService#snapshots We're needlessly wrapping a `SnapshotMissingException` which itself is a `SnapshotException` when trying to load a missing snapshot. This leads to failure #47442 which expects a `SnapshotMissingException` in this case. Closes #47442
elastic · Oct 3, 2019 · 3c011aa · 3c011aa
1 parent e1a3017
commit 3c011aa
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 0 deletions.
diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java
@@ -206,6 +206,9 @@ public List<SnapshotInfo> snapshots(final String repositoryName, final List<Snap
                 if (ignoreUnavailable) {
                     logger.warn(() -> new ParameterizedMessage("failed to get snapshot [{}]", snapshotId), ex);
                 } else {
+                    if (ex instanceof SnapshotException) {
+                        throw ex;
+                    }
                     throw new SnapshotException(repositoryName, snapshotId, "Snapshot could not be read", ex);
                 }
             }

diff --git a/server/src/test/java/org/elasticsearch/snapshots/SnapshotStatusApisIT.java b/server/src/test/java/org/elasticsearch/snapshots/SnapshotStatusApisIT.java
@@ -21,19 +21,26 @@
 import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionFuture;
 import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse;
+import org.elasticsearch.action.admin.cluster.snapshots.get.GetSnapshotsRequest;
+import org.elasticsearch.action.admin.cluster.snapshots.get.GetSnapshotsResponse;
 import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotStatus;
 import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotsStatusRequest;
 import org.elasticsearch.client.Client;
 import org.elasticsearch.cluster.SnapshotsInProgress;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.TimeValue;
+import org.elasticsearch.core.internal.io.IOUtils;
+import org.elasticsearch.repositories.blobstore.BlobStoreRepository;
 
+import java.io.IOException;
+import java.nio.file.Path;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.greaterThan;
+import static org.hamcrest.Matchers.instanceOf;
 
 public class SnapshotStatusApisIT extends AbstractSnapshotIntegTestCase {
 
@@ -110,4 +117,24 @@ public void testStatusAPICallInProgressSnapshot() throws Exception {
         logger.info("--> wait for snapshot to finish");
         createSnapshotResponseActionFuture.actionGet();
     }
+
+    public void testExceptionOnMissingSnapBlob() throws IOException {
+        disableRepoConsistencyCheck("This test intentionally corrupts the repository");
+
+        logger.info("--> creating repository");
+        final Path repoPath = randomRepoPath();
+        assertAcked(client().admin().cluster().preparePutRepository("test-repo").setType("fs").setSettings(
+            Settings.builder().put("location", repoPath).build()));
+
+        logger.info("--> snapshot");
+        final CreateSnapshotResponse response =
+            client().admin().cluster().prepareCreateSnapshot("test-repo", "test-snap").setWaitForCompletion(true).get();
+
+        logger.info("--> delete snap-${uuid}.dat file for this snapshot to simulate concurrent delete");
+        IOUtils.rm(repoPath.resolve(BlobStoreRepository.SNAPSHOT_PREFIX + response.getSnapshotInfo().snapshotId().getUUID() + ".dat"));
+
+        GetSnapshotsResponse snapshotsResponse = client().admin().cluster()
+            .getSnapshots(new GetSnapshotsRequest(new String[] {"test-repo"}, new String[] {"test-snap"})).actionGet();
+        assertThat(snapshotsResponse.getFailedResponses().get("test-repo"), instanceOf(SnapshotMissingException.class));
+    }
 }