From 7f5befd95e19badf8b911df5c28a08613b0a9cd0 Mon Sep 17 00:00:00 2001 From: Igor Motov Date: Sun, 13 Apr 2014 19:20:50 -0400 Subject: [PATCH] Add Partial snapshot state Currently even if some shards of the snapshot are not snapshotted successfully, the snapshot is still marked as "SUCCESS". Users may miss the fact the there are shard failures present in the snapshot and think that snapshot was completed. This change adds a new snapshot state "PARTIAL" that provides a quick indication that the snapshot was only partially successful. Closes #5792 --- .../TransportSnapshotsStatusAction.java | 3 ++ .../blobstore/BlobStoreRepository.java | 6 +++- .../blobstore/BlobStoreSnapshot.java | 11 ++++++- .../snapshots/RestoreService.java | 2 +- .../snapshots/SnapshotState.java | 32 ++++++++++++++++--- .../DedicatedClusterSnapshotRestoreTests.java | 2 +- .../SharedClusterSnapshotRestoreTests.java | 1 + 7 files changed, 48 insertions(+), 9 deletions(-) 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 cec4be3537673..cdb5a67b8b274 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 @@ -206,6 +206,9 @@ private SnapshotsStatusResponse buildResponse(SnapshotsStatusRequest request, Im state = SnapshotMetaData.State.FAILED; break; case SUCCESS: + case PARTIAL: + // Translating both PARTIAL and SUCCESS to SUCCESS for now + // TODO: add the differentiation on the metadata level in the next major release state = SnapshotMetaData.State.SUCCESS; break; default: diff --git a/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index 554789241f5f3..f5119aa0c925b 100644 --- a/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -312,7 +312,11 @@ public Snapshot finalizeSnapshot(SnapshotId snapshotId, String failure, int tota String blobName = snapshotBlobName(snapshotId); BlobStoreSnapshot.Builder updatedSnapshot = BlobStoreSnapshot.builder().snapshot(snapshot); if (failure == null) { - updatedSnapshot.success(); + if (shardFailures.isEmpty()) { + updatedSnapshot.success(); + } else { + updatedSnapshot.partial(); + } updatedSnapshot.failures(totalShards, shardFailures); } else { updatedSnapshot.failed(failure); diff --git a/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreSnapshot.java b/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreSnapshot.java index a6e174e9fd2b4..08a07c43b9fff 100644 --- a/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreSnapshot.java +++ b/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreSnapshot.java @@ -20,7 +20,6 @@ package org.elasticsearch.repositories.blobstore; import com.google.common.collect.ImmutableList; -import com.google.common.primitives.Longs; import org.elasticsearch.Version; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -290,6 +289,16 @@ public Builder success() { return this; } + /** + * Marks snapshot as partially successful + * + * @return this builder + */ + public Builder partial() { + this.state = SnapshotState.PARTIAL; + return this; + } + /** * Marks snapshot as failed and saves failure reason * diff --git a/src/main/java/org/elasticsearch/snapshots/RestoreService.java b/src/main/java/org/elasticsearch/snapshots/RestoreService.java index 2d04296e220b5..6188917b64eee 100644 --- a/src/main/java/org/elasticsearch/snapshots/RestoreService.java +++ b/src/main/java/org/elasticsearch/snapshots/RestoreService.java @@ -119,7 +119,7 @@ public void restoreSnapshot(final RestoreRequest request, final RestoreSnapshotL final MetaData metaData = repository.readSnapshotMetaData(snapshotId, filteredIndices); // Make sure that we can restore from this snapshot - if (snapshot.state() != SnapshotState.SUCCESS) { + if (!snapshot.state().restorable()) { throw new SnapshotRestoreException(snapshotId, "unsupported snapshot state [" + snapshot.state() + "]"); } if (Version.CURRENT.before(snapshot.version())) { diff --git a/src/main/java/org/elasticsearch/snapshots/SnapshotState.java b/src/main/java/org/elasticsearch/snapshots/SnapshotState.java index 88a2a98650fa9..51ca694b84f10 100644 --- a/src/main/java/org/elasticsearch/snapshots/SnapshotState.java +++ b/src/main/java/org/elasticsearch/snapshots/SnapshotState.java @@ -28,20 +28,30 @@ public enum SnapshotState { /** * Snapshot process has started */ - IN_PROGRESS((byte) 0), + IN_PROGRESS((byte) 0, false, false), /** * Snapshot process completed successfully */ - SUCCESS((byte) 1), + SUCCESS((byte) 1, true, true), /** * Snapshot failed */ - FAILED((byte) 2); + FAILED((byte) 2, true, false), + /** + * Snapshot was partial successful + */ + PARTIAL((byte) 3, true, true); private byte value; - private SnapshotState(byte value) { + private boolean completed; + + private boolean restorable; + + private SnapshotState(byte value, boolean completed, boolean restorable) { this.value = value; + this.completed = completed; + this.restorable = restorable; } /** @@ -59,7 +69,17 @@ public byte value() { * @return true if snapshot completed, false otherwise */ public boolean completed() { - return this == SUCCESS || this == FAILED; + return completed; + } + + + /** + * Returns true if snapshot can be restored (at least partially) + * + * @return true if snapshot can be restored, false otherwise + */ + public boolean restorable() { + return restorable; } /** @@ -76,6 +96,8 @@ public static SnapshotState fromValue(byte value) { return SUCCESS; case 2: return FAILED; + case 3: + return PARTIAL; default: throw new ElasticsearchIllegalArgumentException("No snapshot state for value [" + value + "]"); } diff --git a/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreTests.java b/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreTests.java index 9040dd4e10490..42341df765637 100644 --- a/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreTests.java +++ b/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreTests.java @@ -238,7 +238,7 @@ public void restoreIndexWithMissingShards() throws Exception { assertThat(createSnapshotResponse.getSnapshotInfo().totalShards(), equalTo(12)); assertThat(createSnapshotResponse.getSnapshotInfo().successfulShards(), lessThan(12)); assertThat(createSnapshotResponse.getSnapshotInfo().successfulShards(), greaterThan(6)); - assertThat(client().admin().cluster().prepareGetSnapshots("test-repo").setSnapshots("test-snap-2").execute().actionGet().getSnapshots().get(0).state(), equalTo(SnapshotState.SUCCESS)); + assertThat(client().admin().cluster().prepareGetSnapshots("test-repo").setSnapshots("test-snap-2").execute().actionGet().getSnapshots().get(0).state(), equalTo(SnapshotState.PARTIAL)); assertAcked(client().admin().indices().prepareClose("test-idx-1", "test-idx-2").execute().actionGet()); diff --git a/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreTests.java b/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreTests.java index bd0522cbed2c5..0cce6cfa1ee7d 100644 --- a/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreTests.java +++ b/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreTests.java @@ -409,6 +409,7 @@ public void dataFileFailureDuringSnapshotTest() throws Exception { GetSnapshotsResponse getSnapshotsResponse = client.admin().cluster().prepareGetSnapshots("test-repo").addSnapshots("test-snap").get(); assertThat(getSnapshotsResponse.getSnapshots().size(), equalTo(1)); SnapshotInfo snapshotInfo = getSnapshotsResponse.getSnapshots().get(0); + assertThat(snapshotInfo.state(), equalTo(SnapshotState.PARTIAL)); assertThat(snapshotInfo.shardFailures().size(), greaterThan(0)); assertThat(snapshotInfo.totalShards(), greaterThan(snapshotInfo.successfulShards()));