Skip to content

Commit 225ebb6

Browse files
committed
Ensure no snapshotted commit when close engine (#38663)
With this change, we can automatically detect an implementation that acquires an index commit but fails to release.
1 parent f04bd4a commit 225ebb6

File tree

3 files changed

+27
-2
lines changed

3 files changed

+27
-2
lines changed

server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2492,6 +2492,7 @@ public void testConcurrentWritesAndCommits() throws Exception {
24922492
prevLocalCheckpoint = localCheckpoint;
24932493
prevMaxSeqNo = maxSeqNo;
24942494
}
2495+
IOUtils.close(commits);
24952496
}
24962497
}
24972498

server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseIT.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ public void testRetentionLeasesSyncedOnAdd() throws Exception {
105105

106106
// check retention leases have been committed on the primary
107107
final RetentionLeases primaryCommittedRetentionLeases = RetentionLeases.decodeRetentionLeases(
108-
primary.acquireLastIndexCommit(false).getIndexCommit().getUserData().get(Engine.RETENTION_LEASES));
108+
primary.commitStats().getUserData().get(Engine.RETENTION_LEASES));
109109
assertThat(currentRetentionLeases, equalTo(RetentionLeases.toMap(primaryCommittedRetentionLeases)));
110110

111111
// check current retention leases have been synced to all replicas
@@ -120,7 +120,7 @@ public void testRetentionLeasesSyncedOnAdd() throws Exception {
120120

121121
// check retention leases have been committed on the replica
122122
final RetentionLeases replicaCommittedRetentionLeases = RetentionLeases.decodeRetentionLeases(
123-
replica.acquireLastIndexCommit(false).getIndexCommit().getUserData().get(Engine.RETENTION_LEASES));
123+
replica.commitStats().getUserData().get(Engine.RETENTION_LEASES));
124124
assertThat(currentRetentionLeases, equalTo(RetentionLeases.toMap(replicaCommittedRetentionLeases)));
125125
}
126126
}

test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@
8787
import org.elasticsearch.index.engine.CommitStats;
8888
import org.elasticsearch.index.engine.DocIdSeqNoAndTerm;
8989
import org.elasticsearch.index.engine.Engine;
90+
import org.elasticsearch.index.engine.EngineTestCase;
9091
import org.elasticsearch.index.engine.InternalEngine;
9192
import org.elasticsearch.index.seqno.SeqNoStats;
9293
import org.elasticsearch.index.seqno.SequenceNumbers;
@@ -1254,6 +1255,7 @@ public void beforeIndexDeletion() throws Exception {
12541255
//check that shards that have same sync id also contain same number of documents
12551256
assertSameSyncIdSameDocs();
12561257
assertOpenTranslogReferences();
1258+
assertNoSnapshottedIndexCommit();
12571259
}
12581260

12591261
private void assertSameSyncIdSameDocs() {
@@ -1324,6 +1326,28 @@ private void assertOpenTranslogReferences() throws Exception {
13241326
});
13251327
}
13261328

1329+
private void assertNoSnapshottedIndexCommit() throws Exception {
1330+
assertBusy(() -> {
1331+
final Collection<NodeAndClient> nodesAndClients = nodes.values();
1332+
for (NodeAndClient nodeAndClient : nodesAndClients) {
1333+
IndicesService indexServices = getInstance(IndicesService.class, nodeAndClient.name);
1334+
for (IndexService indexService : indexServices) {
1335+
for (IndexShard indexShard : indexService) {
1336+
try {
1337+
Engine engine = IndexShardTestCase.getEngine(indexShard);
1338+
if (engine instanceof InternalEngine) {
1339+
assertFalse(indexShard.routingEntry().toString() + " has unreleased snapshotted index commits",
1340+
EngineTestCase.hasSnapshottedCommits(engine));
1341+
}
1342+
} catch (AlreadyClosedException ignored) {
1343+
1344+
}
1345+
}
1346+
}
1347+
}
1348+
});
1349+
}
1350+
13271351
/**
13281352
* Asserts that the document history in Lucene index is consistent with Translog's on every index shard of the cluster.
13291353
* This assertion might be expensive, thus we prefer not to execute on every test but only interesting tests.

0 commit comments

Comments
 (0)