Skip to content

Commit

Permalink
Improve the error message when attempting to snapshot a closed index
Browse files Browse the repository at this point in the history 
Currently the error message is the same when index is closed and when it is missing shards. This commit will generate a specific failure message when a user tries to create a snapshot of a closed index.

Related to #10579
  • Loading branch information
@imotov
imotov committed Apr 20, 2015
1 parent fe61fd0 commit 4a3df35
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 17 deletions.
44 changes: 33 additions & 11 deletions src/main/java/org/elasticsearch/snapshots/SnapshotsService.java
View file
Expand Up @@ -38,6 +38,7 @@
import org.elasticsearch.cluster.routing.RoutingTable;
import org.elasticsearch.cluster.routing.ShardRouting;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.component.AbstractLifecycleComponent;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.io.stream.StreamInput;
Expand Down Expand Up @@ -299,11 +300,25 @@ public ClusterState execute(ClusterState currentState) {
// Replace the snapshot that was just created
ImmutableMap<ShardId, SnapshotMetaData.ShardSnapshotStatus> shards = shards(currentState, snapshot.indices());
if (!partial) {
Set<String> indicesWithMissingShards = indicesWithMissingShards(shards);
if (indicesWithMissingShards != null) {
Tuple<Set<String>, Set<String>> indicesWithMissingShards = indicesWithMissingShards(shards, currentState.metaData());
Set<String> missing = indicesWithMissingShards.v1();
Set<String> closed = indicesWithMissingShards.v2();
if (missing.isEmpty() == false || closed.isEmpty() == false) {
StringBuilder failureMessage = new StringBuilder();
updatedSnapshot = new SnapshotMetaData.Entry(snapshot.snapshotId(), snapshot.includeGlobalState(), State.FAILED, snapshot.indices(), shards);
entries.add(updatedSnapshot);
failure = "Indices don't have primary shards +[" + indicesWithMissingShards + "]";
if (missing.isEmpty() == false ) {
failureMessage.append("Indices don't have primary shards ");
failureMessage.append(missing);
}
if (closed.isEmpty() == false ) {
if (failureMessage.length() > 0) {
failureMessage.append("; ");
}
failureMessage.append("Indices are closed ");
failureMessage.append(closed);
}
failure = failureMessage.toString();
continue;
}
}
Expand Down Expand Up @@ -864,22 +879,24 @@ private boolean completed(Collection<SnapshotMetaData.ShardSnapshotStatus> shard
}

/**
* Returns list of indices with missing shards
* Returns list of indices with missing shards, and list of indices that are closed
*
* @param shards list of shard statuses
* @return list of failed indices
* @return list of failed and closed indices
*/
private Set<String> indicesWithMissingShards(ImmutableMap<ShardId, SnapshotMetaData.ShardSnapshotStatus> shards) {
Set<String> indices = null;
private Tuple<Set<String>, Set<String>> indicesWithMissingShards(ImmutableMap<ShardId, SnapshotMetaData.ShardSnapshotStatus> shards, MetaData metaData) {
Set<String> missing = newHashSet();
Set<String> closed = newHashSet();
for (ImmutableMap.Entry<ShardId, SnapshotMetaData.ShardSnapshotStatus> entry : shards.entrySet()) {
if (entry.getValue().state() == State.MISSING) {
if (indices == null) {
indices = newHashSet();
if (metaData.hasIndex(entry.getKey().getIndex()) && metaData.index(entry.getKey().getIndex()).getState() == IndexMetaData.State.CLOSE) {
closed.add(entry.getKey().getIndex());
} else {
missing.add(entry.getKey().getIndex());
}
indices.add(entry.getKey().getIndex());
}
}
return indices;
return new Tuple<>(missing, closed);
}

/**
Expand Down Expand Up @@ -1208,6 +1225,11 @@ private ImmutableMap<ShardId, SnapshotMetaData.ShardSnapshotStatus> shards(Clust
if (indexMetaData == null) {
// The index was deleted before we managed to start the snapshot - mark it as missing.
builder.put(new ShardId(index, 0), new SnapshotMetaData.ShardSnapshotStatus(null, State.MISSING, "missing index"));
} else if (indexMetaData.getState() == IndexMetaData.State.CLOSE) {
for (int i = 0; i < indexMetaData.numberOfShards(); i++) {
ShardId shardId = new ShardId(index, i);
builder.put(shardId, new SnapshotMetaData.ShardSnapshotStatus(null, State.MISSING, "index is closed"));
}
} else {
IndexRoutingTable indexRoutingTable = clusterState.getRoutingTable().index(index);
for (int i = 0; i < indexMetaData.numberOfShards(); i++) {
Expand Down
26 changes: 20 additions & 6 deletions src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreTests.java
View file
Expand Up @@ -407,12 +407,18 @@ public void restoreIndexWithMissingShards() throws Exception {
.put("number_of_replicas", 0)));
ensureGreen("test-idx-all");

logger.info("--> create an index that will be closed");
assertAcked(prepareCreate("test-idx-closed", 1, settingsBuilder().put("number_of_shards", 4).put("number_of_replicas", 0)));
ensureGreen("test-idx-closed");

logger.info("--> indexing some data into test-idx-all");
for (int i = 0; i < 100; i++) {
index("test-idx-all", "doc", Integer.toString(i), "foo", "bar" + i);
index("test-idx-closed", "doc", Integer.toString(i), "foo", "bar" + i);
}
refresh();
assertThat(client().prepareCount("test-idx-all").get().getCount(), equalTo(100L));
assertAcked(client().admin().indices().prepareClose("test-idx-closed"));

logger.info("--> create an index that will have no allocated shards");
assertAcked(prepareCreate("test-idx-none", 1, settingsBuilder().put("number_of_shards", 6)
Expand All @@ -426,13 +432,19 @@ public void restoreIndexWithMissingShards() throws Exception {
assertThat(putRepositoryResponse.isAcknowledged(), equalTo(true));

logger.info("--> start snapshot with default settings - should fail");
CreateSnapshotResponse createSnapshotResponse = client().admin().cluster().prepareCreateSnapshot("test-repo", "test-snap-1").setWaitForCompletion(true).execute().actionGet();

CreateSnapshotResponse createSnapshotResponse = client().admin().cluster().prepareCreateSnapshot("test-repo", "test-snap-1")
.setIndices("test-idx-all", "test-idx-none", "test-idx-some", "test-idx-closed")
.setWaitForCompletion(true).execute().actionGet();
assertThat(createSnapshotResponse.getSnapshotInfo().state(), equalTo(SnapshotState.FAILED));
assertThat(createSnapshotResponse.getSnapshotInfo().reason(), containsString("Indices don't have primary shards"));
assertThat(createSnapshotResponse.getSnapshotInfo().reason(), containsString("; Indices are closed [test-idx-closed]"));


if (randomBoolean()) {
logger.info("checking snapshot completion using status");
client().admin().cluster().prepareCreateSnapshot("test-repo", "test-snap-2").setWaitForCompletion(false).setPartial(true).execute().actionGet();
client().admin().cluster().prepareCreateSnapshot("test-repo", "test-snap-2")
.setIndices("test-idx-all", "test-idx-none", "test-idx-some", "test-idx-closed")
.setWaitForCompletion(false).setPartial(true).execute().actionGet();
awaitBusy(new Predicate<Object>() {
@Override
public boolean apply(Object o) {
Expand All @@ -450,7 +462,7 @@ public boolean apply(Object o) {
assertThat(snapshotStatuses.size(), equalTo(1));
SnapshotStatus snapshotStatus = snapshotStatuses.get(0);
logger.info("State: [{}], Reason: [{}]", createSnapshotResponse.getSnapshotInfo().state(), createSnapshotResponse.getSnapshotInfo().reason());
assertThat(snapshotStatus.getShardsStats().getTotalShards(), equalTo(18));
assertThat(snapshotStatus.getShardsStats().getTotalShards(), equalTo(22));
assertThat(snapshotStatus.getShardsStats().getDoneShards(), lessThan(12));
assertThat(snapshotStatus.getShardsStats().getDoneShards(), greaterThan(6));

Expand All @@ -471,9 +483,11 @@ public boolean apply(Object o) {
});
} else {
logger.info("checking snapshot completion using wait_for_completion flag");
createSnapshotResponse = client().admin().cluster().prepareCreateSnapshot("test-repo", "test-snap-2").setWaitForCompletion(true).setPartial(true).execute().actionGet();
createSnapshotResponse = client().admin().cluster().prepareCreateSnapshot("test-repo", "test-snap-2")
.setIndices("test-idx-all", "test-idx-none", "test-idx-some", "test-idx-closed")
.setWaitForCompletion(true).setPartial(true).execute().actionGet();
logger.info("State: [{}], Reason: [{}]", createSnapshotResponse.getSnapshotInfo().state(), createSnapshotResponse.getSnapshotInfo().reason());
assertThat(createSnapshotResponse.getSnapshotInfo().totalShards(), equalTo(18));
assertThat(createSnapshotResponse.getSnapshotInfo().totalShards(), equalTo(22));
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.PARTIAL));
Expand Down
12 changes: 12 additions & 0 deletions src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreTests.java
View file
Expand Up @@ -942,6 +942,18 @@ public void snapshotClosedIndexTest() throws Exception {

logger.info("--> deleting snapshot");
client.admin().cluster().prepareDeleteSnapshot("test-repo", "test-snap").get();

logger.info("--> snapshot with closed index");
createSnapshotResponse = client.admin().cluster().prepareCreateSnapshot("test-repo", "test-snap").setWaitForCompletion(true).setIndices("test-idx", "test-idx-closed").get();
assertThat(createSnapshotResponse.getSnapshotInfo().indices().size(), equalTo(2));
assertThat(createSnapshotResponse.getSnapshotInfo().state(), equalTo(SnapshotState.FAILED));
assertThat(createSnapshotResponse.getSnapshotInfo().reason(), containsString("Indices are closed [test-idx-closed]"));
for(SnapshotShardFailure failure : createSnapshotResponse.getSnapshotInfo().shardFailures()) {
assertThat(failure.reason(), containsString("index is closed"));
}

logger.info("--> deleting snapshot");
client.admin().cluster().prepareDeleteSnapshot("test-repo", "test-snap").get();
}

@Test
Expand Down

0 comments on commit 4a3df35

Please sign in to comment.