From 1f61850807deea0a1d1aff457f0d75dce9f905f5 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Tue, 14 Jan 2020 14:05:27 +0200 Subject: [PATCH 1/3] Done --- .../TransportCleanupRepositoryAction.java | 7 ++- .../snapshots/SnapshotsService.java | 5 +- .../CorruptedBlobStoreRepositoryIT.java | 56 ++++++++++++++++++- .../snapshots/mockstore/MockRepository.java | 18 ++++++ 4 files changed, 81 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java index 1b2381ed89ec5..116410ee74827 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java @@ -175,7 +175,12 @@ private void cleanupRepo(String repositoryName, ActionListener repositoryDataListener = new StepListener<>(); - repository.getRepositoryData(repositoryDataListener); + try { + repository.getRepositoryData(repositoryDataListener); + } catch(Exception e) { + listener.onFailure(e); + return; + } repositoryDataListener.whenComplete(repositoryData -> { final long repositoryStateId = repositoryData.getGenId(); logger.info("Running cleanup operations on repository [{}][{}]", repositoryName, repositoryStateId); diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index 9340e1508999c..7abe8ff41faef 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -269,7 +269,7 @@ public void createSnapshot(final CreateSnapshotRequest request, final ActionList validate(repositoryName, snapshotName); final SnapshotId snapshotId = new SnapshotId(snapshotName, UUIDs.randomBase64UUID()); // new UUID for the snapshot final StepListener repositoryDataListener = new StepListener<>(); - repositoriesService.repository(repositoryName).getRepositoryData(repositoryDataListener); + getRepositoryData(repositoryName, repositoryDataListener); repositoryDataListener.whenComplete(repositoryData -> { final boolean hasOldFormatSnapshots = hasOldVersionSnapshots(repositoryName, repositoryData, null); clusterService.submitStateUpdateTask("create_snapshot [" + snapshotName + ']', new ClusterStateUpdateTask() { @@ -1199,8 +1199,7 @@ public void clusterStateProcessed(String source, ClusterState oldState, ClusterS public void deleteSnapshot(final String repositoryName, final String snapshotName, final ActionListener listener, final boolean immediatePriority) { // First, look for the snapshot in the repository - final Repository repository = repositoriesService.repository(repositoryName); - repository.getRepositoryData(ActionListener.wrap(repositoryData -> { + getRepositoryData(repositoryName, ActionListener.wrap(repositoryData -> { Optional matchedEntry = repositoryData.getSnapshotIds() .stream() .filter(s -> s.getName().equals(snapshotName)) diff --git a/server/src/test/java/org/elasticsearch/snapshots/CorruptedBlobStoreRepositoryIT.java b/server/src/test/java/org/elasticsearch/snapshots/CorruptedBlobStoreRepositoryIT.java index 9abb107521042..3ae29e19dd3e4 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/CorruptedBlobStoreRepositoryIT.java +++ b/server/src/test/java/org/elasticsearch/snapshots/CorruptedBlobStoreRepositoryIT.java @@ -18,8 +18,11 @@ */ package org.elasticsearch.snapshots; +import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionRunnable; import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse; +import org.elasticsearch.action.admin.cluster.snapshots.get.GetSnapshotsResponse; import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.client.Client; import org.elasticsearch.cluster.ClusterState; @@ -34,11 +37,14 @@ import org.elasticsearch.repositories.RepositoryData; import org.elasticsearch.repositories.RepositoryException; import org.elasticsearch.repositories.blobstore.BlobStoreRepository; +import org.elasticsearch.snapshots.mockstore.MockRepository; import org.elasticsearch.threadpool.ThreadPool; +import org.hamcrest.Matchers; import java.nio.file.Files; import java.nio.file.Path; import java.util.Locale; +import java.util.concurrent.CountDownLatch; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.hamcrest.Matchers.containsString; @@ -105,7 +111,55 @@ public void testConcurrentlyChangeRepositoryContents() throws Exception { logger.info("--> make sure snapshot doesn't exist"); expectThrows(SnapshotMissingException.class, () -> client.admin().cluster().prepareGetSnapshots(repoName) - .addSnapshots(snapshot).get().getSnapshots(repoName)); + .addSnapshots(snapshot).get().getSnapshots(repoName)); + } + + public void testRetrievingRepositoryDataThrows() throws Exception { + disableRepoConsistencyCheck("This test does not create any data in the repository."); + String randomNodeName = randomFrom(internalCluster().getNodeNames()); + Client client = client(randomNodeName); + + Path repo = randomRepoPath(); + final String repoName = "test-mock-repo"; + logger.info("--> creating repository at {}", repo.toAbsolutePath()); + assertAcked(client.admin().cluster().preparePutRepository(repoName) + .setType("mock").setSettings(Settings.builder() + .put("location", repo) + .put("compress", false) + .put(BlobStoreRepository.ALLOW_CONCURRENT_MODIFICATION.getKey(), true) + .put("chunk_size", randomIntBetween(100, 1000), ByteSizeUnit.BYTES))); + + ((MockRepository) internalCluster().getCurrentMasterNodeInstance(RepositoriesService.class) + .repository(repoName)).setThrowOnGetRepositoryData(true); + + ElasticsearchException e = expectThrows(ElasticsearchException.class, () -> { + client.admin().cluster().prepareCreateSnapshot(repoName, "test-snap").setWaitForCompletion(true).get(); + }); + assertThat(e.getMessage(), Matchers.is("Expected test exception")); + + GetSnapshotsResponse snapshotsResponse = client.admin().cluster().prepareGetSnapshots(repoName).get(); + assertThat(snapshotsResponse.getSuccessfulResponses().size(), Matchers.is(0)); + assertThat(snapshotsResponse.getFailedResponses().size(), Matchers.is(1)); + assertThat(snapshotsResponse.getFailedResponses().get(repoName).getMessage(), + Matchers.is("Expected test exception")); + + e = expectThrows(ElasticsearchException.class, () -> { + client.admin().cluster().prepareDeleteSnapshot(repoName, "test-snap").get(); + }); + assertThat(e.getMessage(), Matchers.is("Expected test exception")); + + e = expectThrows(ElasticsearchException.class, () -> { + client.admin().cluster().prepareRestoreSnapshot(repoName, "test-snap").get(); + }); + assertThat(e.getMessage(), Matchers.is("Expected test exception")); + + e = expectThrows(ElasticsearchException.class, () -> { + client.admin().cluster().prepareCleanupRepository(repoName).get(); + }); + assertThat(e.getMessage(), Matchers.is("Expected test exception")); + + ((MockRepository) internalCluster().getCurrentMasterNodeInstance(RepositoriesService.class) + .repository(repoName)).setThrowOnGetRepositoryData(true); } public void testConcurrentlyChangeRepositoryContentsInBwCMode() throws Exception { diff --git a/test/framework/src/main/java/org/elasticsearch/snapshots/mockstore/MockRepository.java b/test/framework/src/main/java/org/elasticsearch/snapshots/mockstore/MockRepository.java index 7b21c878599e9..90a2e6e3802e8 100644 --- a/test/framework/src/main/java/org/elasticsearch/snapshots/mockstore/MockRepository.java +++ b/test/framework/src/main/java/org/elasticsearch/snapshots/mockstore/MockRepository.java @@ -24,6 +24,7 @@ import org.apache.logging.log4j.Logger; import org.apache.lucene.index.CorruptIndexException; import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.action.ActionListener; import org.elasticsearch.cluster.metadata.RepositoryMetaData; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.blobstore.BlobContainer; @@ -40,6 +41,7 @@ import org.elasticsearch.env.Environment; import org.elasticsearch.plugins.RepositoryPlugin; import org.elasticsearch.repositories.Repository; +import org.elasticsearch.repositories.RepositoryData; import org.elasticsearch.repositories.fs.FsRepository; import java.io.IOException; @@ -56,6 +58,7 @@ import java.util.Random; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; import java.util.stream.Collectors; @@ -115,6 +118,8 @@ public long getFailureCount() { private volatile boolean blocked = false; + private AtomicBoolean throwOnGetRepositoryData = new AtomicBoolean(false); + public MockRepository(RepositoryMetaData metadata, Environment environment, NamedXContentRegistry namedXContentRegistry, ClusterService clusterService) { super(overrideSettings(metadata, environment), environment, namedXContentRegistry, clusterService); @@ -131,6 +136,19 @@ public MockRepository(RepositoryMetaData metadata, Environment environment, logger.info("starting mock repository with random prefix {}", randomPrefix); } + @Override + public void getRepositoryData(ActionListener listener) { + if (throwOnGetRepositoryData.get()) { + throw new ElasticsearchException("Expected test exception"); + } else { + super.getRepositoryData(listener); + } + } + + public void setThrowOnGetRepositoryData(boolean throwOnGet) { + this.throwOnGetRepositoryData.set(throwOnGet); + } + @Override public RepositoryMetaData getMetadata() { return overrideSettings(super.getMetadata(), env); From 51e091b5c56777cfbe1a3df7dd06436b5b8199ee Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Tue, 14 Jan 2020 14:06:33 +0200 Subject: [PATCH 2/3] nit --- .../snapshots/CorruptedBlobStoreRepositoryIT.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/snapshots/CorruptedBlobStoreRepositoryIT.java b/server/src/test/java/org/elasticsearch/snapshots/CorruptedBlobStoreRepositoryIT.java index 3ae29e19dd3e4..c980c2e2f5084 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/CorruptedBlobStoreRepositoryIT.java +++ b/server/src/test/java/org/elasticsearch/snapshots/CorruptedBlobStoreRepositoryIT.java @@ -44,7 +44,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Locale; -import java.util.concurrent.CountDownLatch; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.hamcrest.Matchers.containsString; @@ -111,7 +110,7 @@ public void testConcurrentlyChangeRepositoryContents() throws Exception { logger.info("--> make sure snapshot doesn't exist"); expectThrows(SnapshotMissingException.class, () -> client.admin().cluster().prepareGetSnapshots(repoName) - .addSnapshots(snapshot).get().getSnapshots(repoName)); + .addSnapshots(snapshot).get().getSnapshots(repoName)); } public void testRetrievingRepositoryDataThrows() throws Exception { @@ -159,7 +158,7 @@ public void testRetrievingRepositoryDataThrows() throws Exception { assertThat(e.getMessage(), Matchers.is("Expected test exception")); ((MockRepository) internalCluster().getCurrentMasterNodeInstance(RepositoriesService.class) - .repository(repoName)).setThrowOnGetRepositoryData(true); + .repository(repoName)).setThrowOnGetRepositoryData(false); } public void testConcurrentlyChangeRepositoryContentsInBwCMode() throws Exception { From b74e3c4b7be3873cfd31f7626d6ce1a496051daa Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Tue, 14 Jan 2020 15:02:25 +0200 Subject: [PATCH 3/3] Checkstyle --- .../elasticsearch/snapshots/CorruptedBlobStoreRepositoryIT.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/snapshots/CorruptedBlobStoreRepositoryIT.java b/server/src/test/java/org/elasticsearch/snapshots/CorruptedBlobStoreRepositoryIT.java index c980c2e2f5084..ab6eba9db5198 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/CorruptedBlobStoreRepositoryIT.java +++ b/server/src/test/java/org/elasticsearch/snapshots/CorruptedBlobStoreRepositoryIT.java @@ -19,7 +19,6 @@ package org.elasticsearch.snapshots; import org.elasticsearch.ElasticsearchException; -import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionRunnable; import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse; import org.elasticsearch.action.admin.cluster.snapshots.get.GetSnapshotsResponse;