Skip to content

Commit 8e34393

Browse files
authored
Fail primary term and generation listeners on a closed shard (#122713)
If a shard has been closed, we should quickly bail out and fail all waiting primary term and generation listeners. Otherwise, the engine implementation may try to successfully to complete the provided listeners and perform operations on an already closed shard and cause some unexpected errors.
1 parent f220aba commit 8e34393

File tree

2 files changed

+25
-6
lines changed

2 files changed

+25
-6
lines changed

server/src/main/java/org/elasticsearch/index/shard/IndexShard.java

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4491,14 +4491,17 @@ public void waitForEngineOrClosedShard(ActionListener<Void> listener) {
44914491
}
44924492

44934493
/**
4494-
* Registers a listener for an event when the shard advances to the provided primary term and segment generation
4494+
* Registers a listener for an event when the shard advances to the provided primary term and segment generation.
4495+
* Completes the listener with a {@link IndexShardClosedException} if the shard is closed.
44954496
*/
44964497
public void waitForPrimaryTermAndGeneration(long primaryTerm, long segmentGeneration, ActionListener<Long> listener) {
4497-
waitForEngineOrClosedShard(
4498-
listener.delegateFailureAndWrap(
4499-
(l, ignored) -> getEngine().addPrimaryTermAndGenerationListener(primaryTerm, segmentGeneration, l)
4500-
)
4501-
);
4498+
waitForEngineOrClosedShard(listener.delegateFailureAndWrap((l, ignored) -> {
4499+
if (state == IndexShardState.CLOSED) {
4500+
l.onFailure(new IndexShardClosedException(shardId));
4501+
} else {
4502+
getEngine().addPrimaryTermAndGenerationListener(primaryTerm, segmentGeneration, l);
4503+
}
4504+
}));
45024505
}
45034506

45044507
/**

server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.apache.lucene.util.BytesRef;
2828
import org.apache.lucene.util.Constants;
2929
import org.elasticsearch.ElasticsearchException;
30+
import org.elasticsearch.ExceptionsHelper;
3031
import org.elasticsearch.action.ActionListener;
3132
import org.elasticsearch.action.admin.indices.flush.FlushRequest;
3233
import org.elasticsearch.action.admin.indices.forcemerge.ForceMergeRequest;
@@ -3334,6 +3335,21 @@ public void testWaitForClosedListener() throws IOException {
33343335
assertThat("listener should have been called", called.get(), equalTo(true));
33353336
}
33363337

3338+
public void testWaitForPrimaryTermAndGenerationFailsForClosedShard() throws IOException {
3339+
Settings settings = indexSettings(IndexVersion.current(), 1, 1).build();
3340+
IndexMetadata metadata = IndexMetadata.builder("test").putMapping("""
3341+
{ "properties": { "foo": { "type": "text"}}}""").settings(settings).primaryTerm(0, 1).build();
3342+
IndexShard initializingShard = newShard(new ShardId(metadata.getIndex(), 0), true, "n1", metadata, null);
3343+
3344+
var future = new PlainActionFuture<Long>();
3345+
initializingShard.waitForPrimaryTermAndGeneration(0L, 0L, future);
3346+
3347+
assertFalse("waitForPrimaryTermAndGeneration should be waiting", future.isDone());
3348+
closeShards(initializingShard);
3349+
// Should bail out earlier without calling the engine
3350+
assertNotNull(ExceptionsHelper.unwrap(expectThrows(Exception.class, future::get), IndexShardClosedException.class));
3351+
}
3352+
33373353
public void testRecoverFromLocalShard() throws IOException {
33383354
Settings settings = indexSettings(IndexVersion.current(), 1, 1).build();
33393355
IndexMetadata metadata = IndexMetadata.builder("source")

0 commit comments

Comments
 (0)