Browse Source

Improve get-snapshots message for unreadable repository (#128273) (#128662)

Today if the get-snapshots API cannot access one of the repositories we
return a fairly low-level message about the problem, perhaps something
like `Could not determine repository generation from root blobs`. This
message is shown verbatim in the Kibana UI so users need something a
little more descriptive. With this commit we wrap the exception in one
that indicates the problem in terms that users are more likely to
understand.

Relates #128208
David Turner 4 months ago
parent
commit
45078216f9

+ 5 - 0
docs/changelog/128273.yaml

@@ -0,0 +1,5 @@
+pr: 128273
+summary: Improve get-snapshots message for unreadable repository
+area: Snapshot/Restore
+type: enhancement
+issues: []

+ 44 - 0
server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java

@@ -40,7 +40,9 @@ import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.core.Predicates;
 import org.elasticsearch.repositories.RepositoriesService;
 import org.elasticsearch.repositories.RepositoryData;
+import org.elasticsearch.repositories.RepositoryException;
 import org.elasticsearch.repositories.RepositoryMissingException;
+import org.elasticsearch.repositories.blobstore.BlobStoreRepository;
 import org.elasticsearch.repositories.fs.FsRepository;
 import org.elasticsearch.search.sort.SortOrder;
 import org.elasticsearch.test.ESTestCase;
@@ -633,6 +635,48 @@ public class GetSnapshotsIT extends AbstractSnapshotIntegTestCase {
         expectThrows(RepositoryMissingException.class, multiRepoFuture::actionGet);
     }
 
+    public void testRetrievingSnapshotsWhenRepositoryIsUnreadable() throws Exception {
+        final String repoName = randomIdentifier();
+        final Path repoPath = randomRepoPath();
+        createRepository(
+            repoName,
+            "fs",
+            Settings.builder().put("location", repoPath).put(BlobStoreRepository.CACHE_REPOSITORY_DATA.getKey(), false)
+        );
+        createNSnapshots(repoName, randomIntBetween(1, 3));
+
+        try {
+            try (var directoryStream = Files.newDirectoryStream(repoPath)) {
+                for (final var directoryEntry : directoryStream) {
+                    if (Files.isRegularFile(directoryEntry) && directoryEntry.getFileName().toString().startsWith("index-")) {
+                        Files.writeString(directoryEntry, "invalid");
+                    }
+                }
+            }
+
+            final var repositoryException = safeAwaitAndUnwrapFailure(
+                RepositoryException.class,
+                GetSnapshotsResponse.class,
+                l -> clusterAdmin().prepareGetSnapshots(TEST_REQUEST_TIMEOUT, repoName)
+                    .setSort(SnapshotSortKey.NAME)
+                    .setIgnoreUnavailable(randomBoolean())
+                    .execute(l)
+            );
+            assertEquals(
+                Strings.format("[%s] cannot retrieve snapshots list from this repository", repoName),
+                repositoryException.getMessage()
+            );
+            assertEquals(
+                Strings.format("[%s] Unexpected exception when loading repository data", repoName),
+                repositoryException.getCause().getMessage()
+            );
+        } finally {
+            safeAwait(
+                l -> clusterAdmin().prepareDeleteRepository(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT, repoName).execute(l.map(v -> null))
+            );
+        }
+    }
+
     // Create a snapshot that is guaranteed to have a unique start time and duration for tests around ordering by either.
     // Don't use this with more than 3 snapshots on platforms with low-resolution clocks as the durations could always collide there
     // causing an infinite loop

+ 15 - 1
server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java

@@ -38,6 +38,7 @@ import org.elasticsearch.repositories.IndexId;
 import org.elasticsearch.repositories.RepositoriesService;
 import org.elasticsearch.repositories.Repository;
 import org.elasticsearch.repositories.RepositoryData;
+import org.elasticsearch.repositories.RepositoryException;
 import org.elasticsearch.repositories.RepositoryMissingException;
 import org.elasticsearch.repositories.ResolvedRepositories;
 import org.elasticsearch.search.sort.SortOrder;
@@ -285,7 +286,20 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
                         ),
                         repositoryName -> asyncRepositoryContentsListener -> SubscribableListener
 
-                            .<RepositoryData>newForked(l -> maybeGetRepositoryData(repositoryName, l))
+                            .<RepositoryData>newForked(
+                                l -> maybeGetRepositoryData(
+                                    repositoryName,
+                                    l.delegateResponse(
+                                        (ll, e) -> ll.onFailure(
+                                            new RepositoryException(
+                                                repositoryName,
+                                                "cannot retrieve snapshots list from this repository",
+                                                e
+                                            )
+                                        )
+                                    )
+                                )
+                            )
                             .andThenApply(repositoryData -> {
                                 assert ThreadPool.assertCurrentThreadPool(ThreadPool.Names.MANAGEMENT);
                                 cancellableTask.ensureNotCancelled();