Skip to content

Commit 1bc218a

Browse files
committed
Fail primary term and generation listeners on a closed shard
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 54eab9d commit 1bc218a

File tree

2 files changed

+23
-5
lines changed

2 files changed

+23
-5
lines changed

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4494,11 +4494,13 @@ public void waitForEngineOrClosedShard(ActionListener<Void> listener) {
44944494
* Registers a listener for an event when the shard advances to the provided primary term and segment generation
44954495
*/
44964496
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-
);
4497+
waitForEngineOrClosedShard(listener.delegateFailureAndWrap((l, ignored) -> {
4498+
if (state == IndexShardState.CLOSED) {
4499+
l.onFailure(new IndexShardClosedException(shardId));
4500+
} else {
4501+
getEngine().addPrimaryTermAndGenerationListener(primaryTerm, segmentGeneration, l);
4502+
}
4503+
}));
45024504
}
45034505

45044506
/**

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3334,6 +3334,22 @@ public void testWaitForClosedListener() throws IOException {
33343334
assertThat("listener should have been called", called.get(), equalTo(true));
33353335
}
33363336

3337+
public void testWaitForPrimaryTermAndGenerationFailsForClosedShard() throws IOException {
3338+
Settings settings = indexSettings(IndexVersion.current(), 1, 1).build();
3339+
IndexMetadata metadata = IndexMetadata.builder("test").putMapping("""
3340+
{ "properties": { "foo": { "type": "text"}}}""").settings(settings).primaryTerm(0, 1).build();
3341+
IndexShard primary = newShard(new ShardId(metadata.getIndex(), 0), true, "n1", metadata, null);
3342+
3343+
var exception = new AtomicReference<Exception>();
3344+
ActionListener<Long> listener = ActionListener.wrap(l -> { assert false : l; }, e -> exception.set(e));
3345+
primary.waitForPrimaryTermAndGeneration(0L, 0L, listener);
3346+
3347+
assertNull("waitForPrimaryTermAndGeneration should be waiting", exception.get());
3348+
closeShards(primary);
3349+
// Should bail out earlier without calling the engine
3350+
assertThat(exception.get(), instanceOf(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)