Browse Source

Prevent closing wrong search contexts in compute service (#128222)

The search context variable should be inside the loop, not outside; 
otherwise, it may close the previous search context when the target
shard is unavailable, potentially leaving search contexts without
references. This bug was introduced in unreleased versions (9.1 and
8.19). I think we need to refactor DataNodeComputeHandler to allow
writing unit tests for these cases easier, but testSearchWhileRelocating
should cover this issue.

Closes #127188
Nhat Nguyen 4 months ago
parent
commit
e230c01fb0

+ 0 - 3
muted-tests.yml

@@ -393,9 +393,6 @@ tests:
 - class: org.elasticsearch.packaging.test.DockerTests
   method: test026InstallBundledRepositoryPluginsViaConfigFile
   issue: https://github.com/elastic/elasticsearch/issues/127158
-- class: org.elasticsearch.xpack.esql.plugin.DataNodeRequestSenderIT
-  method: testSearchWhileRelocating
-  issue: https://github.com/elastic/elasticsearch/issues/127188
 - class: org.elasticsearch.xpack.remotecluster.CrossClusterEsqlRCS2EnrichUnavailableRemotesIT
   method: testEsqlEnrichWithSkipUnavailable
   issue: https://github.com/elastic/elasticsearch/issues/127368

+ 1 - 1
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/DataNodeComputeHandler.java

@@ -328,8 +328,8 @@ final class DataNodeComputeHandler implements TransportRequestHandler<DataNodeRe
             }
             final var doAcquire = ActionRunnable.supply(listener, () -> {
                 final List<SearchContext> searchContexts = new ArrayList<>(targetShards.size());
-                SearchContext context = null;
                 for (IndexShard shard : targetShards) {
+                    SearchContext context = null;
                     try {
                         var aliasFilter = aliasFilters.getOrDefault(shard.shardId().getIndex(), AliasFilter.EMPTY);
                         var shardRequest = new ShardSearchRequest(