Rework ShardSearchContextId to explain use of searcher id better (#135233)

cbuescher · web-flow · commit ac0cf9541881 · 2025-09-25T13:48:40.000+02:00
When a ShardSearchContextId has a searcher id, this indicates that the
searcher backing the context is stable so we can retry a shard on a
different node if the original context is gone (e.g. due to node
restarts). We use the searcher ids only for PIT contexts and only if the
underlying engine supports it (e.g. currently only FrozenEngine and
ReadOnlyEngine).
This change moves the related decision logic into ShardSearchContextId itself
because there is no need to expose the search id once its set. By
renaming the access methods we can also communicate the intended use of
these ids better.
diff --git a/server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java b/server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java
@@ -81,6 +81,7 @@
 import org.elasticsearch.search.builder.SearchSourceBuilder;
 import org.elasticsearch.search.internal.AliasFilter;
 import org.elasticsearch.search.internal.SearchContext;
+import org.elasticsearch.search.internal.ShardSearchContextId;
 import org.elasticsearch.search.profile.SearchProfileResults;
 import org.elasticsearch.search.profile.SearchProfileShardResult;
 import org.elasticsearch.tasks.Task;
@@ -1271,7 +1272,8 @@ static List<SearchShardIterator> getRemoteShardsIteratorFromPointInTime(
                     // Otherwise, we add the shard iterator without a target node, allowing a partial search failure to
                     // be thrown when a search phase attempts to access it.
                     targetNodes.add(perNode.getNode());
-                    if (perNode.getSearchContextId().getSearcherId() != null) {
+                    ShardSearchContextId shardSearchContextId = perNode.getSearchContextId();
+                    if (shardSearchContextId != null && shardSearchContextId.isRetryable()) {
                         for (String node : group.allocatedNodes()) {
                             if (node.equals(perNode.getNode()) == false) {
                                 targetNodes.add(node);
@@ -1947,7 +1949,8 @@ static List<SearchShardIterator> getLocalShardsIteratorFromPointInTime(
                         if (projectState.cluster().nodes().nodeExists(perNode.getNode())) {
                             targetNodes.add(perNode.getNode());
                         }
-                        if (perNode.getSearchContextId().getSearcherId() != null) {
+                        ShardSearchContextId shardSearchContextId = perNode.getSearchContextId();
+                        if (shardSearchContextId.isRetryable()) {
                             for (ShardRouting shard : shards) {
                                 if (shard.currentNodeId().equals(perNode.getNode()) == false) {
                                     targetNodes.add(shard.currentNodeId());
diff --git a/server/src/main/java/org/elasticsearch/search/SearchService.java b/server/src/main/java/org/elasticsearch/search/SearchService.java
@@ -1264,18 +1264,18 @@ private ReaderContext findReaderContext(ShardSearchContextId id, TransportReques
     }
 
     final ReaderContext createOrGetReaderContext(ShardSearchRequest request) {
+        ShardSearchContextId readerId = request.readerId();
         if (request.readerId() != null) {
             try {
-                return findReaderContext(request.readerId(), request);
+                return findReaderContext(readerId, request);
             } catch (SearchContextMissingException e) {
-                final String searcherId = request.readerId().getSearcherId();
-                if (searcherId == null) {
+                if (readerId.isRetryable() == false) {
                     throw e;
                 }
                 final IndexService indexService = indicesService.indexServiceSafe(request.shardId().getIndex());
                 final IndexShard shard = indexService.getShard(request.shardId().id());
                 final Engine.SearcherSupplier searcherSupplier = shard.acquireSearcherSupplier();
-                if (searcherId.equals(searcherSupplier.getSearcherId()) == false) {
+                if (readerId.sameSearcherIdsAs(searcherSupplier.getSearcherId()) == false) {
                     searcherSupplier.close();
                     throw e;
                 }
@@ -1298,8 +1298,8 @@ final ReaderContext createAndPutReaderContext(
         ReaderContext readerContext = null;
         Releasable decreaseScrollContexts = null;
         try {
-            final ShardSearchContextId id = new ShardSearchContextId(sessionId, idGenerator.incrementAndGet());
             if (request.scroll() != null) {
+                final ShardSearchContextId id = new ShardSearchContextId(sessionId, idGenerator.incrementAndGet());
                 decreaseScrollContexts = openScrollContexts::decrementAndGet;
                 if (openScrollContexts.incrementAndGet() > maxOpenScrollContext) {
                     throw new TooManyScrollContextsException(maxOpenScrollContext, MAX_OPEN_SCROLL_CONTEXT.getKey());
@@ -1308,6 +1308,7 @@ final ReaderContext createAndPutReaderContext(
                 readerContext.addOnClose(decreaseScrollContexts);
                 decreaseScrollContexts = null;
             } else {
+                final ShardSearchContextId id = new ShardSearchContextId(sessionId, idGenerator.incrementAndGet(), reader.getSearcherId());
                 readerContext = new ReaderContext(id, indexService, shard, reader, keepAliveInMillis, true);
             }
             reader = null;
@@ -1399,7 +1400,7 @@ public SearchContext createSearchContext(ShardSearchRequest request, TimeValue t
         final IndexService indexService = indicesService.indexServiceSafe(request.shardId().getIndex());
         final IndexShard indexShard = indexService.getShard(request.shardId().getId());
         final Engine.SearcherSupplier reader = indexShard.acquireSearcherSupplier();
-        final ShardSearchContextId id = new ShardSearchContextId(sessionId, idGenerator.incrementAndGet());
+        final ShardSearchContextId id = new ShardSearchContextId(sessionId, idGenerator.incrementAndGet(), reader.getSearcherId());
         try (ReaderContext readerContext = new ReaderContext(id, indexService, indexShard, reader, -1L, true)) {
             DefaultSearchContext searchContext = createSearchContext(readerContext, request, timeout, ResultsType.NONE);
             searchContext.addReleasable(readerContext.markAsUsed(0L));
@@ -2037,8 +2038,7 @@ static CanMatchShardResponse canMatch(CanMatchContext canMatchContext, boolean c
                     }
                     searcher = readerContext.acquireSearcher(Engine.CAN_MATCH_SEARCH_SOURCE);
                 } catch (SearchContextMissingException e) {
-                    final String searcherId = canMatchContext.request.readerId().getSearcherId();
-                    if (searcherId == null) {
+                    if (canMatchContext.request.readerId().isRetryable() == false) {
                         return new CanMatchShardResponse(true, null);
                     }
                     if (queryStillMatchesAfterRewrite(
@@ -2048,7 +2048,7 @@ static CanMatchShardResponse canMatch(CanMatchContext canMatchContext, boolean c
                         return new CanMatchShardResponse(false, null);
                     }
                     final Engine.SearcherSupplier searcherSupplier = canMatchContext.getShard().acquireSearcherSupplier();
-                    if (searcherId.equals(searcherSupplier.getSearcherId()) == false) {
+                    if (canMatchContext.request.readerId().sameSearcherIdsAs(searcherSupplier.getSearcherId()) == false) {
                         searcherSupplier.close();
                         return new CanMatchShardResponse(true, null);
                     }
diff --git a/server/src/main/java/org/elasticsearch/search/internal/LegacyReaderContext.java b/server/src/main/java/org/elasticsearch/search/internal/LegacyReaderContext.java
@@ -36,7 +36,7 @@ public LegacyReaderContext(
         super(id, indexService, indexShard, reader, keepAliveInMillis, false);
         assert shardSearchRequest.readerId() == null;
         assert shardSearchRequest.keepAlive() == null;
-        assert id.getSearcherId() == null : "Legacy reader context must not have searcher id";
+        assert id.isRetryable() == false : "Legacy reader context is not retryable";
         this.shardSearchRequest = Objects.requireNonNull(shardSearchRequest, "ShardSearchRequest must be provided");
         if (shardSearchRequest.scroll() != null) {
             // Search scroll requests are special, they don't hold indices names so we have
diff --git a/server/src/main/java/org/elasticsearch/search/internal/ShardSearchContextId.java b/server/src/main/java/org/elasticsearch/search/internal/ShardSearchContextId.java
@@ -21,7 +21,6 @@ public final class ShardSearchContextId implements Writeable {
     private final long id;
     private final String searcherId;
 
-    // TODO: Remove this constructor
     public ShardSearchContextId(String sessionId, long id) {
         this(sessionId, id, null);
     }
@@ -43,7 +42,6 @@ public void writeTo(StreamOutput out) throws IOException {
         out.writeLong(id);
         out.writeString(sessionId);
         out.writeOptionalString(searcherId);
-
     }
 
     public String getSessionId() {
@@ -54,8 +52,12 @@ public long getId() {
         return id;
     }
 
-    public String getSearcherId() {
-        return searcherId;
+    public boolean isRetryable() {
+        return this.searcherId != null;
+    }
+
+    public boolean sameSearcherIdsAs(String otherSearcherId) {
+        return this.isRetryable() && this.searcherId.equals(otherSearcherId);
     }
 
     @Override
diff --git a/server/src/test/java/org/elasticsearch/action/search/TransportSearchActionTests.java b/server/src/test/java/org/elasticsearch/action/search/TransportSearchActionTests.java
@@ -1702,7 +1702,7 @@ public void testLocalShardIteratorFromPointInTime() {
             final ShardId shardId = new ShardId(indexMetadata.getIndex(), id);
             final SearchShardIterator shardIterator = shardIterators.get(id);
             final SearchContextIdForNode context = contexts.get(shardId);
-            if (context.getSearchContextId().getSearcherId() == null) {
+            if (context.getSearchContextId().isRetryable() == false) {
                 assertThat(shardIterator.getTargetNodeIds(), hasSize(1));
             } else {
                 final List<String> targetNodes = clusterState.routingTable(project)
