From 3d23a71fa7f3664209028e1ac377fe72609ac169 Mon Sep 17 00:00:00 2001 From: Igor Motov Date: Sun, 13 Apr 2014 18:45:37 -0400 Subject: [PATCH] Fix snapshot status with empty repository The snapshot status command with empty repository should return current status of currently running snapshots in all repositories. Fixes #5790 --- .../status/SnapshotsStatusRequest.java | 4 +-- .../TransportSnapshotsStatusAction.java | 3 ++- .../client/ClusterAdminClient.java | 5 ++++ .../support/AbstractClusterAdminClient.java | 5 ++++ .../status/RestSnapshotsStatusAction.java | 2 +- .../snapshots/SnapshotsService.java | 2 +- .../SharedClusterSnapshotRestoreTests.java | 26 +++++++++++++++---- 7 files changed, 37 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/status/SnapshotsStatusRequest.java b/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/status/SnapshotsStatusRequest.java index b5b3815d5d3..c84ad073bd8 100644 --- a/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/status/SnapshotsStatusRequest.java +++ b/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/status/SnapshotsStatusRequest.java @@ -34,11 +34,11 @@ import static org.elasticsearch.action.ValidateActions.addValidationError; */ public class SnapshotsStatusRequest extends MasterNodeOperationRequest { - private String repository; + private String repository = "_all"; private String[] snapshots = Strings.EMPTY_ARRAY; - SnapshotsStatusRequest() { + public SnapshotsStatusRequest() { } /** diff --git a/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/status/TransportSnapshotsStatusAction.java b/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/status/TransportSnapshotsStatusAction.java index f8f1b15d955..cec4be35376 100644 --- a/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/status/TransportSnapshotsStatusAction.java +++ b/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/status/TransportSnapshotsStatusAction.java @@ -87,7 +87,8 @@ public class TransportSnapshotsStatusAction extends TransportMasterNodeOperation ImmutableList currentSnapshots = snapshotsService.currentSnapshots(request.repository(), request.snapshots()); if (currentSnapshots.isEmpty()) { - buildResponse(request, currentSnapshots, null); + listener.onResponse(buildResponse(request, currentSnapshots, null)); + return; } Set nodesIds = newHashSet(); diff --git a/src/main/java/org/elasticsearch/client/ClusterAdminClient.java b/src/main/java/org/elasticsearch/client/ClusterAdminClient.java index 46cebf77301..6f5cd88c6a5 100644 --- a/src/main/java/org/elasticsearch/client/ClusterAdminClient.java +++ b/src/main/java/org/elasticsearch/client/ClusterAdminClient.java @@ -447,4 +447,9 @@ public interface ClusterAdminClient { */ SnapshotsStatusRequestBuilder prepareSnapshotStatus(String repository); + /** + * Get snapshot status. + */ + SnapshotsStatusRequestBuilder prepareSnapshotStatus(); + } diff --git a/src/main/java/org/elasticsearch/client/support/AbstractClusterAdminClient.java b/src/main/java/org/elasticsearch/client/support/AbstractClusterAdminClient.java index eb8a9d47b56..a855435cdd9 100644 --- a/src/main/java/org/elasticsearch/client/support/AbstractClusterAdminClient.java +++ b/src/main/java/org/elasticsearch/client/support/AbstractClusterAdminClient.java @@ -419,4 +419,9 @@ public abstract class AbstractClusterAdminClient implements InternalClusterAdmin public SnapshotsStatusRequestBuilder prepareSnapshotStatus(String repository) { return new SnapshotsStatusRequestBuilder(this, repository); } + + @Override + public SnapshotsStatusRequestBuilder prepareSnapshotStatus() { + return new SnapshotsStatusRequestBuilder(this); + } } diff --git a/src/main/java/org/elasticsearch/rest/action/admin/cluster/snapshots/status/RestSnapshotsStatusAction.java b/src/main/java/org/elasticsearch/rest/action/admin/cluster/snapshots/status/RestSnapshotsStatusAction.java index a4afab5622f..eb901172f03 100644 --- a/src/main/java/org/elasticsearch/rest/action/admin/cluster/snapshots/status/RestSnapshotsStatusAction.java +++ b/src/main/java/org/elasticsearch/rest/action/admin/cluster/snapshots/status/RestSnapshotsStatusAction.java @@ -49,7 +49,7 @@ public class RestSnapshotsStatusAction extends BaseRestHandler { @Override public void handleRequest(final RestRequest request, final RestChannel channel) { - String repository = request.param("repository"); + String repository = request.param("repository", "_all"); String[] snapshots = request.paramAsStringArray("snapshot", Strings.EMPTY_ARRAY); if (snapshots.length == 1 && "_all".equalsIgnoreCase(snapshots[0])) { snapshots = Strings.EMPTY_ARRAY; diff --git a/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index 9afbd227e61..78dcc8e8734 100644 --- a/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -382,7 +382,7 @@ public class SnapshotsService extends AbstractComponent implements ClusterStateL if (snapshotMetaData == null || snapshotMetaData.entries().isEmpty()) { return ImmutableList.of(); } - if (repository == null) { + if ("_all".equals(repository)) { return snapshotMetaData.entries(); } if (snapshotMetaData.entries().size() == 1) { diff --git a/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreTests.java b/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreTests.java index 1ce1f41576c..9cc2aa3d100 100644 --- a/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreTests.java +++ b/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreTests.java @@ -1036,12 +1036,8 @@ public class SharedClusterSnapshotRestoreTests extends AbstractSnapshotTests { logger.info("--> waiting for block to kick in"); waitForBlock(blockedNode, "test-repo", TimeValue.timeValueSeconds(60)); - logger.info("--> execution was blocked on node [{}], checking snapshot status", blockedNode); + logger.info("--> execution was blocked on node [{}], checking snapshot status with specified repository and snapshot", blockedNode); SnapshotsStatusResponse response = client.admin().cluster().prepareSnapshotStatus("test-repo").execute().actionGet(); - - logger.info("--> unblocking blocked node"); - unblockNode(blockedNode); - assertThat(response.getSnapshots().size(), equalTo(1)); SnapshotStatus snapshotStatus = response.getSnapshots().get(0); assertThat(snapshotStatus.getState(), equalTo(SnapshotMetaData.State.STARTED)); @@ -1053,6 +1049,22 @@ public class SharedClusterSnapshotRestoreTests extends AbstractSnapshotTests { } } + logger.info("--> checking snapshot status for all currently running and snapshot with empty repository", blockedNode); + response = client.admin().cluster().prepareSnapshotStatus().execute().actionGet(); + assertThat(response.getSnapshots().size(), equalTo(1)); + snapshotStatus = response.getSnapshots().get(0); + assertThat(snapshotStatus.getState(), equalTo(SnapshotMetaData.State.STARTED)); + // We blocked the node during data write operation, so at least one shard snapshot should be in STARTED stage + assertThat(snapshotStatus.getShardsStats().getStartedShards(), greaterThan(0)); + for( SnapshotIndexShardStatus shardStatus : snapshotStatus.getIndices().get("test-idx")) { + if (shardStatus.getStage() == SnapshotIndexShardStage.STARTED) { + assertThat(shardStatus.getNodeId(), notNullValue()); + } + } + + logger.info("--> unblocking blocked node"); + unblockNode(blockedNode); + SnapshotInfo snapshotInfo = waitForCompletion("test-repo", "test-snap", TimeValue.timeValueSeconds(600)); logger.info("Number of failed shards [{}]", snapshotInfo.shardFailures().size()); logger.info("--> done"); @@ -1069,6 +1081,10 @@ public class SharedClusterSnapshotRestoreTests extends AbstractSnapshotTests { assertThat(indexStatus.getShardsStats().getDoneShards(), equalTo(snapshotInfo.successfulShards())); assertThat(indexStatus.getShards().size(), equalTo(snapshotInfo.totalShards())); + logger.info("--> checking snapshot status after it is done with empty repository", blockedNode); + response = client.admin().cluster().prepareSnapshotStatus().execute().actionGet(); + assertThat(response.getSnapshots().size(), equalTo(0)); + try { client.admin().cluster().prepareSnapshotStatus("test-repo").addSnapshots("test-snap-doesnt-exist").execute().actionGet(); fail();