From 92234ff63d73635152993ca5ec259e0bdaf60d53 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 25 Nov 2019 07:56:33 +0100 Subject: [PATCH 1/2] Wrap Exceptions Thrown by GetSnapshotsResponse As asked for in #43462. I think this is a worthwhile change if only to make tests easier to debug. Closes #43462 --- .../snapshots/get/GetSnapshotsResponse.java | 4 +- .../fs/FsBlobStoreRepositoryIT.java | 3 +- .../DedicatedClusterSnapshotRestoreIT.java | 3 +- .../SharedClusterSnapshotRestoreIT.java | 46 +++++++++---------- .../ESBlobStoreRepositoryIntegTestCase.java | 11 ++++- .../slm/SLMSnapshotBlockingIntegTests.java | 8 +++- 6 files changed, 44 insertions(+), 31 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponse.java index 595ed753c8359..7cbb298710fc8 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponse.java @@ -162,7 +162,7 @@ public List getSnapshots(String repo) { if (error == null) { throw new IllegalArgumentException("No such repository"); } - throw error; + throw new ElasticsearchException(error); } /** @@ -265,4 +265,4 @@ public static GetSnapshotsResponse fromXContent(XContentParser parser) throws IO public String toString() { return Strings.toString(this); } -} \ No newline at end of file +} diff --git a/server/src/test/java/org/elasticsearch/repositories/fs/FsBlobStoreRepositoryIT.java b/server/src/test/java/org/elasticsearch/repositories/fs/FsBlobStoreRepositoryIT.java index b79d250eceefd..04fe45232b566 100644 --- a/server/src/test/java/org/elasticsearch/repositories/fs/FsBlobStoreRepositoryIT.java +++ b/server/src/test/java/org/elasticsearch/repositories/fs/FsBlobStoreRepositoryIT.java @@ -29,7 +29,6 @@ import java.nio.file.Files; import java.nio.file.NoSuchFileException; import java.nio.file.Path; -import java.util.concurrent.ExecutionException; import java.util.stream.Stream; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; @@ -55,7 +54,7 @@ protected Settings repositorySettings() { return settings.build(); } - public void testMissingDirectoriesNotCreatedInReadonlyRepository() throws IOException, ExecutionException, InterruptedException { + public void testMissingDirectoriesNotCreatedInReadonlyRepository() throws IOException, InterruptedException { final String repoName = randomName(); final Path repoPath = randomRepoPath(); diff --git a/server/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java b/server/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java index a11fb9d13bc04..817aa6f3ab45f 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java +++ b/server/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java @@ -70,6 +70,7 @@ import org.elasticsearch.node.Node; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.repositories.RepositoryMissingException; +import org.elasticsearch.repositories.blobstore.ESBlobStoreRepositoryIntegTestCase; import org.elasticsearch.rest.AbstractRestChannel; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestRequest; @@ -490,7 +491,7 @@ public void testSnapshotWithStuckNode() throws Exception { } logger.info("--> making sure that snapshot no longer exists"); - expectThrows(SnapshotMissingException.class, + ESBlobStoreRepositoryIntegTestCase.expectSnapshotMissingException( () -> client().admin().cluster().prepareGetSnapshots("test-repo").setSnapshots("test-snap") .execute().actionGet().getSnapshots("test-repo")); diff --git a/server/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java b/server/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java index 0ec69bdaa0424..758afee8547fd 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java +++ b/server/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java @@ -120,6 +120,7 @@ import static org.elasticsearch.index.IndexSettings.INDEX_SOFT_DELETES_SETTING; import static org.elasticsearch.index.query.QueryBuilders.matchQuery; import static org.elasticsearch.index.shard.IndexShardTests.getEngineFromShard; +import static org.elasticsearch.repositories.blobstore.ESBlobStoreRepositoryIntegTestCase.expectSnapshotMissingException; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAllSuccessful; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; @@ -1405,7 +1406,7 @@ public void testGetSnapshotsMultipleRepos() throws Exception { .get(); for (String repo : repoList) { - expectThrows(SnapshotMissingException.class, () -> getSnapshotsResponse2.getSnapshots(repo)); + expectSnapshotMissingException(() -> getSnapshotsResponse2.getSnapshots(repo)); } @@ -1470,8 +1471,8 @@ public void testDeleteSnapshotWithMissingIndexAndShardMetadata() throws Exceptio logger.info("--> make sure snapshot doesn't exist"); - expectThrows(SnapshotMissingException.class, () -> client.admin().cluster().prepareGetSnapshots("test-repo") - .addSnapshots("test-snap-1").get().getSnapshots("test-repo")); + expectSnapshotMissingException( + () -> client.admin().cluster().prepareGetSnapshots("test-repo").addSnapshots("test-snap-1").get().getSnapshots("test-repo")); for (String index : indices) { assertTrue(Files.notExists(indicesPath.resolve(indexIds.get(index).getId()))); @@ -1510,7 +1511,7 @@ public void testDeleteSnapshotWithMissingMetadata() throws Exception { client.admin().cluster().prepareDeleteSnapshot("test-repo", "test-snap-1").get(); logger.info("--> make sure snapshot doesn't exist"); - expectThrows(SnapshotMissingException.class, () -> client.admin().cluster().prepareGetSnapshots("test-repo") + expectSnapshotMissingException(() -> client.admin().cluster().prepareGetSnapshots("test-repo") .addSnapshots("test-snap-1").get().getSnapshots("test-repo")); } @@ -1547,9 +1548,10 @@ public void testDeleteSnapshotWithCorruptedSnapshotFile() throws Exception { client.admin().cluster().prepareDeleteSnapshot("test-repo", "test-snap-1").get(); logger.info("--> make sure snapshot doesn't exist"); - expectThrows(SnapshotMissingException.class, + final ElasticsearchException ex = expectThrows(ElasticsearchException.class, () -> client.admin().cluster().prepareGetSnapshots("test-repo").addSnapshots("test-snap-1").get(). getSnapshots("test-repo")); + assertThat(ExceptionsHelper.unwrap(ex, SnapshotMissingException.class), notNullValue()); logger.info("--> make sure that we can create the snapshot again"); createSnapshotResponse = client.admin().cluster().prepareCreateSnapshot("test-repo", "test-snap-1") @@ -1606,10 +1608,10 @@ public void testDeleteSnapshotWithCorruptedGlobalState() throws Exception { assertThat(snapshotStatusResponse.getSnapshots().get(0).getSnapshot().getSnapshotId().getName(), equalTo("test-snap")); assertAcked(client().admin().cluster().prepareDeleteSnapshot("test-repo", "test-snap").get()); - expectThrows(SnapshotMissingException.class, () -> client().admin().cluster() - .prepareGetSnapshots("test-repo").addSnapshots("test-snap").get().getSnapshots("test-repo")); - assertThrows(client().admin().cluster().prepareSnapshotStatus("test-repo").addSnapshots("test-snap"), - SnapshotMissingException.class); + expectSnapshotMissingException( + () -> client().admin().cluster().prepareGetSnapshots("test-repo").addSnapshots("test-snap").get().getSnapshots("test-repo")); + expectSnapshotMissingException( + () -> client().admin().cluster().prepareSnapshotStatus("test-repo").addSnapshots("test-snap").get()); createSnapshotResponse = client().admin().cluster().prepareCreateSnapshot("test-repo", "test-snap") .setIncludeGlobalState(true) @@ -2756,13 +2758,10 @@ public void testSnapshotName() throws Exception { expectThrows(InvalidSnapshotNameException.class, () -> client.admin().cluster().prepareCreateSnapshot("test-repo", "_foo").get()); - expectThrows(SnapshotMissingException.class, - () -> client.admin().cluster().prepareGetSnapshots("test-repo").setSnapshots("_foo") - .get().getSnapshots("test-repo")); - expectThrows(SnapshotMissingException.class, - () -> client.admin().cluster().prepareDeleteSnapshot("test-repo", "_foo").get()); - expectThrows(SnapshotMissingException.class, - () -> client.admin().cluster().prepareSnapshotStatus("test-repo").setSnapshots("_foo").get()); + expectSnapshotMissingException( + () -> client.admin().cluster().prepareGetSnapshots("test-repo").setSnapshots("_foo").get().getSnapshots("test-repo")); + expectSnapshotMissingException(() -> client.admin().cluster().prepareDeleteSnapshot("test-repo", "_foo").get()); + expectSnapshotMissingException(() -> client.admin().cluster().prepareSnapshotStatus("test-repo").setSnapshots("_foo").get()); } public void testListCorruptedSnapshot() throws Exception { @@ -2811,8 +2810,9 @@ public void testListCorruptedSnapshot() throws Exception { assertThat(snapshotInfos.get(0).state(), equalTo(SnapshotState.SUCCESS)); assertThat(snapshotInfos.get(0).snapshotId().getName(), equalTo("test-snap-1")); - final SnapshotException ex = expectThrows(SnapshotException.class, () -> + final ElasticsearchException e = expectThrows(ElasticsearchException.class, () -> client.admin().cluster().prepareGetSnapshots("test-repo").setIgnoreUnavailable(false).get().getSnapshots("test-repo")); + final SnapshotException ex = (SnapshotException) ExceptionsHelper.unwrap(e, SnapshotException.class); assertThat(ex.getRepositoryName(), equalTo("test-repo")); assertThat(ex.getSnapshotName(), equalTo("test-snap-2")); } @@ -3131,12 +3131,12 @@ public void testGetSnapshotsRequest() throws Exception { .put("wait_after_unblock", 200))); logger.info("--> get snapshots on an empty repository"); - expectThrows(SnapshotMissingException.class, () -> client.admin() - .cluster() - .prepareGetSnapshots(repositoryName) - .addSnapshots("non-existent-snapshot") - .get() - .getSnapshots(repositoryName)); + expectSnapshotMissingException(() -> client.admin() + .cluster() + .prepareGetSnapshots(repositoryName) + .addSnapshots("non-existent-snapshot") + .get() + .getSnapshots(repositoryName)); // with ignore unavailable set to true, should not throw an exception GetSnapshotsResponse getSnapshotsResponse = client.admin() .cluster() diff --git a/test/framework/src/main/java/org/elasticsearch/repositories/blobstore/ESBlobStoreRepositoryIntegTestCase.java b/test/framework/src/main/java/org/elasticsearch/repositories/blobstore/ESBlobStoreRepositoryIntegTestCase.java index ccf3853ebe64b..ceaf68798cd3f 100644 --- a/test/framework/src/main/java/org/elasticsearch/repositories/blobstore/ESBlobStoreRepositoryIntegTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/repositories/blobstore/ESBlobStoreRepositoryIntegTestCase.java @@ -19,6 +19,8 @@ package org.elasticsearch.repositories.blobstore; import org.apache.lucene.util.SetOnce; +import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotRequestBuilder; import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse; import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotRequestBuilder; @@ -62,6 +64,13 @@ public static RepositoryData getRepositoryData(Repository repository) { return PlainActionFuture.get(repository::getRepositoryData); } + public static SnapshotMissingException expectSnapshotMissingException(ThrowingRunnable runnable) { + final ElasticsearchException ex = expectThrows(ElasticsearchException.class, runnable); + final Throwable sme = ExceptionsHelper.unwrap(ex, SnapshotMissingException.class); + assertThat(sme, notNullValue()); + return (SnapshotMissingException) sme; + } + protected abstract String repositoryType(); protected Settings repositorySettings() { @@ -153,7 +162,7 @@ public void testSnapshotAndRestore() throws Exception { logger.info("--> delete snapshot {}:{}", repoName, snapshotName); assertAcked(client().admin().cluster().prepareDeleteSnapshot(repoName, snapshotName).get()); - expectThrows(SnapshotMissingException.class, () -> + expectSnapshotMissingException(() -> client().admin().cluster().prepareGetSnapshots(repoName).setSnapshots(snapshotName).get().getSnapshots(repoName)); expectThrows(SnapshotMissingException.class, () -> diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SLMSnapshotBlockingIntegTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SLMSnapshotBlockingIntegTests.java index b59ca988010e0..d0d76b0dc06c9 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SLMSnapshotBlockingIntegTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SLMSnapshotBlockingIntegTests.java @@ -6,6 +6,8 @@ package org.elasticsearch.xpack.slm; +import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.action.ActionFuture; import org.elasticsearch.action.admin.cluster.snapshots.get.GetSnapshotsResponse; import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotStatus; @@ -53,6 +55,7 @@ import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.notNullValue; /** * Tests for Snapshot Lifecycle Management that require a slow or blocked snapshot repo (using {@link MockRepository} @@ -297,7 +300,7 @@ private void testUnsuccessfulSnapshotRetention(boolean partialSuccess) throws Ex .prepareGetSnapshots(REPO).setSnapshots(failedSnapshotName.get()).get(); SnapshotInfo snapshotInfo = snapshotsStatusResponse.getSnapshots(REPO).get(0); assertEquals(expectedUnsuccessfulState, snapshotInfo.state()); - } catch (SnapshotMissingException ex) { + } catch (ElasticsearchException ex) { logger.info("failed to find snapshot {}, retrying", failedSnapshotName); throw new AssertionError(ex); } @@ -358,7 +361,8 @@ private void testUnsuccessfulSnapshotRetention(boolean partialSuccess) throws Ex GetSnapshotsResponse snapshotsStatusResponse = client().admin().cluster() .prepareGetSnapshots(REPO).setSnapshots(failedSnapshotName.get()).get(); assertThat(snapshotsStatusResponse.getSnapshots(REPO), empty()); - } catch (SnapshotMissingException e) { + } catch (ElasticsearchException e) { + assertThat(ExceptionsHelper.unwrap(e, SnapshotMissingException.class), notNullValue()); // This is what we want to happen } logger.info("--> {} snapshot [{}] has been deleted, checking successful snapshot [{}] still exists", From e82550a2ffa602b3c62963c907019435b70a547c Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 26 Nov 2019 15:38:01 +0100 Subject: [PATCH 2/2] drier --- .../snapshots/SharedClusterSnapshotRestoreIT.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java b/server/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java index 758afee8547fd..5d707c37d5d20 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java +++ b/server/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java @@ -1548,10 +1548,8 @@ public void testDeleteSnapshotWithCorruptedSnapshotFile() throws Exception { client.admin().cluster().prepareDeleteSnapshot("test-repo", "test-snap-1").get(); logger.info("--> make sure snapshot doesn't exist"); - final ElasticsearchException ex = expectThrows(ElasticsearchException.class, - () -> client.admin().cluster().prepareGetSnapshots("test-repo").addSnapshots("test-snap-1").get(). - getSnapshots("test-repo")); - assertThat(ExceptionsHelper.unwrap(ex, SnapshotMissingException.class), notNullValue()); + expectSnapshotMissingException(() -> client.admin().cluster().prepareGetSnapshots("test-repo").addSnapshots("test-snap-1").get(). + getSnapshots("test-repo")); logger.info("--> make sure that we can create the snapshot again"); createSnapshotResponse = client.admin().cluster().prepareCreateSnapshot("test-repo", "test-snap-1")