Skip to content

Commit c03f286

Browse files
committed
Abort early on finding duplicate snapshot name in internal structures (#29634)
Adds a check in BlobstoreRepository.snapshot(...) that prevents duplicate snapshot names and fails the snapshot before writing out the new index file. This ensures that you cannot end up in this situation where the index file has duplicate names and cannot be read anymore . Relates to #28906
1 parent 4bafa54 commit c03f286

File tree

2 files changed

+44
-0
lines changed

2 files changed

+44
-0
lines changed

server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1130,6 +1130,11 @@ public void snapshot(final IndexCommit snapshotIndexCommit) {
11301130
BlobStoreIndexShardSnapshots snapshots = tuple.v1();
11311131
int fileListGeneration = tuple.v2();
11321132

1133+
if (snapshots.snapshots().stream().anyMatch(sf -> sf.snapshot().equals(snapshotId.getName()))) {
1134+
throw new IndexShardSnapshotFailedException(shardId,
1135+
"Duplicate snapshot name [" + snapshotId.getName() + "] detected, aborting");
1136+
}
1137+
11331138
final List<BlobStoreIndexShardSnapshot.FileInfo> indexCommitPointFiles = new ArrayList<>();
11341139

11351140
store.incRef();

server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryRestoreTests.java

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.elasticsearch.index.shard.IndexShardState;
3434
import org.elasticsearch.index.shard.IndexShardTestCase;
3535
import org.elasticsearch.index.shard.ShardId;
36+
import org.elasticsearch.index.snapshots.IndexShardSnapshotFailedException;
3637
import org.elasticsearch.index.store.Store;
3738
import org.elasticsearch.index.store.StoreFileMetaData;
3839
import org.elasticsearch.repositories.IndexId;
@@ -48,6 +49,7 @@
4849
import java.util.List;
4950

5051
import static org.elasticsearch.cluster.routing.RecoverySource.StoreRecoverySource.EXISTING_STORE_INSTANCE;
52+
import static org.hamcrest.Matchers.containsString;
5153

5254
/**
5355
* This class tests the behavior of {@link BlobStoreRepository} when it
@@ -126,6 +128,43 @@ public void testRestoreSnapshotWithExistingFiles() throws IOException {
126128
}
127129
}
128130

131+
public void testSnapshotWithConflictingName() throws IOException {
132+
final IndexId indexId = new IndexId(randomAlphaOfLength(10), UUIDs.randomBase64UUID());
133+
final ShardId shardId = new ShardId(indexId.getName(), indexId.getId(), 0);
134+
135+
IndexShard shard = newShard(shardId, true);
136+
try {
137+
// index documents in the shards
138+
final int numDocs = scaledRandomIntBetween(1, 500);
139+
recoverShardFromStore(shard);
140+
for (int i = 0; i < numDocs; i++) {
141+
indexDoc(shard, "doc", Integer.toString(i));
142+
if (rarely()) {
143+
flushShard(shard, false);
144+
}
145+
}
146+
assertDocCount(shard, numDocs);
147+
148+
// snapshot the shard
149+
final Repository repository = createRepository();
150+
final Snapshot snapshot = new Snapshot(repository.getMetadata().name(), new SnapshotId(randomAlphaOfLength(10), "_uuid"));
151+
snapshotShard(shard, snapshot, repository);
152+
final Snapshot snapshotWithSameName = new Snapshot(repository.getMetadata().name(), new SnapshotId(
153+
snapshot.getSnapshotId().getName(), "_uuid2"));
154+
IndexShardSnapshotFailedException isfe = expectThrows(IndexShardSnapshotFailedException.class,
155+
() -> snapshotShard(shard, snapshotWithSameName, repository));
156+
assertThat(isfe.getMessage(), containsString("Duplicate snapshot name"));
157+
} finally {
158+
if (shard != null && shard.state() != IndexShardState.CLOSED) {
159+
try {
160+
shard.close("test", false);
161+
} finally {
162+
IOUtils.close(shard.store());
163+
}
164+
}
165+
}
166+
}
167+
129168
/** Create a {@link Repository} with a random name **/
130169
private Repository createRepository() throws IOException {
131170
Settings settings = Settings.builder().put("location", randomAlphaOfLength(10)).build();

0 commit comments

Comments
 (0)