Skip to content

Commit 6db7984

Browse files
authored
ESQL: Make it a little easier to test LOOKUP (#120540)
* ESQL: Make it a little easier to test LOOKUP This changes the internals of LOOKUP so they don't rely directly on `SearchContext`, instead relying on their own `LookupShardContext` which is easy to build from a `SearchContext`. The advantage is that it's easier to build it *without* a `SearchContext` which makes unit testing a ton easier. * JAVADOC
1 parent 3e6b8bf commit 6db7984

File tree

4 files changed

+72
-27
lines changed

4 files changed

+72
-27
lines changed

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/AbstractLookupService.java

Lines changed: 50 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -130,10 +130,10 @@
130130
* the same number of rows that it was sent no matter how many documents match.
131131
* </p>
132132
*/
133-
abstract class AbstractLookupService<R extends AbstractLookupService.Request, T extends AbstractLookupService.TransportRequest> {
133+
public abstract class AbstractLookupService<R extends AbstractLookupService.Request, T extends AbstractLookupService.TransportRequest> {
134134
private final String actionName;
135135
private final ClusterService clusterService;
136-
private final SearchService searchService;
136+
private final CreateShardContext createShardContext;
137137
private final TransportService transportService;
138138
private final Executor executor;
139139
private final BigArrays bigArrays;
@@ -152,7 +152,7 @@ abstract class AbstractLookupService<R extends AbstractLookupService.Request, T
152152
AbstractLookupService(
153153
String actionName,
154154
ClusterService clusterService,
155-
SearchService searchService,
155+
CreateShardContext createShardContext,
156156
TransportService transportService,
157157
BigArrays bigArrays,
158158
BlockFactory blockFactory,
@@ -161,7 +161,7 @@ abstract class AbstractLookupService<R extends AbstractLookupService.Request, T
161161
) {
162162
this.actionName = actionName;
163163
this.clusterService = clusterService;
164-
this.searchService = searchService;
164+
this.createShardContext = createShardContext;
165165
this.transportService = transportService;
166166
this.executor = transportService.getThreadPool().executor(ThreadPool.Names.SEARCH);
167167
this.bigArrays = bigArrays;
@@ -324,9 +324,8 @@ private void doLookup(T request, CancellableTask task, ActionListener<List<Page>
324324
final List<Releasable> releasables = new ArrayList<>(6);
325325
boolean started = false;
326326
try {
327-
final ShardSearchRequest shardSearchRequest = new ShardSearchRequest(request.shardId, 0, AliasFilter.EMPTY);
328-
final SearchContext searchContext = searchService.createSearchContext(shardSearchRequest, SearchService.NO_TIMEOUT);
329-
releasables.add(searchContext);
327+
LookupShardContext shardContext = createShardContext.create(request.shardId);
328+
releasables.add(shardContext.release);
330329
final LocalCircuitBreaker localBreaker = new LocalCircuitBreaker(
331330
blockFactory.breaker(),
332331
localBreakerSettings.overReservedBytes(),
@@ -364,8 +363,7 @@ private void doLookup(T request, CancellableTask task, ActionListener<List<Page>
364363
}
365364
}
366365
releasables.add(finishPages);
367-
SearchExecutionContext searchExecutionContext = searchContext.getSearchExecutionContext();
368-
QueryList queryList = queryList(request, searchExecutionContext, inputBlock, request.inputDataType);
366+
QueryList queryList = queryList(request, shardContext.executionContext, inputBlock, request.inputDataType);
369367
var warnings = Warnings.createWarnings(
370368
DriverContext.WarningsMode.COLLECT,
371369
request.source.source().getLineNumber(),
@@ -376,11 +374,11 @@ private void doLookup(T request, CancellableTask task, ActionListener<List<Page>
376374
driverContext.blockFactory(),
377375
EnrichQuerySourceOperator.DEFAULT_MAX_PAGE_SIZE,
378376
queryList,
379-
searchExecutionContext.getIndexReader(),
377+
shardContext.context.searcher().getIndexReader(),
380378
warnings
381379
);
382380
releasables.add(queryOperator);
383-
var extractFieldsOperator = extractFieldsOperator(searchContext, driverContext, request.extractFields);
381+
var extractFieldsOperator = extractFieldsOperator(shardContext.context, driverContext, request.extractFields);
384382
releasables.add(extractFieldsOperator);
385383

386384
/*
@@ -403,7 +401,7 @@ private void doLookup(T request, CancellableTask task, ActionListener<List<Page>
403401
List.of(extractFieldsOperator, finishPages),
404402
outputOperator,
405403
Driver.DEFAULT_STATUS_INTERVAL,
406-
Releasables.wrap(searchContext, localBreaker)
404+
Releasables.wrap(shardContext.release, localBreaker)
407405
);
408406
task.addListener(() -> {
409407
String reason = Objects.requireNonNullElse(task.getReasonCancelled(), "task was cancelled");
@@ -440,15 +438,10 @@ public void onFailure(Exception e) {
440438
}
441439

442440
private static Operator extractFieldsOperator(
443-
SearchContext searchContext,
441+
EsPhysicalOperationProviders.ShardContext shardContext,
444442
DriverContext driverContext,
445443
List<NamedExpression> extractFields
446444
) {
447-
EsPhysicalOperationProviders.ShardContext shardContext = new EsPhysicalOperationProviders.DefaultShardContext(
448-
0,
449-
searchContext.getSearchExecutionContext(),
450-
searchContext.request().getAliasFilter()
451-
);
452445
List<ValuesSourceReaderOperator.FieldInfo> fields = new ArrayList<>(extractFields.size());
453446
for (NamedExpression extractField : extractFields) {
454447
BlockLoader loader = shardContext.blockLoader(
@@ -472,7 +465,7 @@ private static Operator extractFieldsOperator(
472465
return new ValuesSourceReaderOperator(
473466
driverContext.blockFactory(),
474467
fields,
475-
List.of(new ValuesSourceReaderOperator.ShardContext(searchContext.searcher().getIndexReader(), searchContext::newSourceLoader)),
468+
List.of(new ValuesSourceReaderOperator.ShardContext(shardContext.searcher().getIndexReader(), shardContext::newSourceLoader)),
476469
0
477470
);
478471
}
@@ -680,4 +673,42 @@ public boolean hasReferences() {
680673
return refs.hasReferences();
681674
}
682675
}
676+
677+
/**
678+
* Create a {@link LookupShardContext} for a locally allocated {@link ShardId}.
679+
*/
680+
public interface CreateShardContext {
681+
LookupShardContext create(ShardId shardId) throws IOException;
682+
683+
static CreateShardContext fromSearchService(SearchService searchService) {
684+
return shardId -> {
685+
ShardSearchRequest shardSearchRequest = new ShardSearchRequest(shardId, 0, AliasFilter.EMPTY);
686+
return LookupShardContext.fromSearchContext(
687+
searchService.createSearchContext(shardSearchRequest, SearchService.NO_TIMEOUT)
688+
);
689+
};
690+
}
691+
}
692+
693+
/**
694+
* {@link AbstractLookupService} uses this to power the queries and field loading that
695+
* it needs to perform to actually do the lookup.
696+
*/
697+
public record LookupShardContext(
698+
EsPhysicalOperationProviders.ShardContext context,
699+
SearchExecutionContext executionContext,
700+
Releasable release
701+
) {
702+
public static LookupShardContext fromSearchContext(SearchContext context) {
703+
return new LookupShardContext(
704+
new EsPhysicalOperationProviders.DefaultShardContext(
705+
0,
706+
context.getSearchExecutionContext(),
707+
context.request().getAliasFilter()
708+
),
709+
context.getSearchExecutionContext(),
710+
context
711+
);
712+
}
713+
}
683714
}

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/EnrichLookupService.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import org.elasticsearch.index.mapper.RangeType;
2424
import org.elasticsearch.index.query.SearchExecutionContext;
2525
import org.elasticsearch.index.shard.ShardId;
26-
import org.elasticsearch.search.SearchService;
2726
import org.elasticsearch.tasks.TaskId;
2827
import org.elasticsearch.transport.TransportService;
2928
import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver;
@@ -48,15 +47,15 @@ public class EnrichLookupService extends AbstractLookupService<EnrichLookupServi
4847

4948
public EnrichLookupService(
5049
ClusterService clusterService,
51-
SearchService searchService,
50+
CreateShardContext createShardContext,
5251
TransportService transportService,
5352
BigArrays bigArrays,
5453
BlockFactory blockFactory
5554
) {
5655
super(
5756
LOOKUP_ACTION_NAME,
5857
clusterService,
59-
searchService,
58+
createShardContext,
6059
transportService,
6160
bigArrays,
6261
blockFactory,

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/LookupFromIndexService.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import org.elasticsearch.index.mapper.MappedFieldType;
2323
import org.elasticsearch.index.query.SearchExecutionContext;
2424
import org.elasticsearch.index.shard.ShardId;
25-
import org.elasticsearch.search.SearchService;
2625
import org.elasticsearch.tasks.TaskId;
2726
import org.elasticsearch.transport.TransportService;
2827
import org.elasticsearch.xpack.esql.EsqlIllegalArgumentException;
@@ -47,15 +46,15 @@ public class LookupFromIndexService extends AbstractLookupService<LookupFromInde
4746

4847
public LookupFromIndexService(
4948
ClusterService clusterService,
50-
SearchService searchService,
49+
CreateShardContext createShardContext,
5150
TransportService transportService,
5251
BigArrays bigArrays,
5352
BlockFactory blockFactory
5453
) {
5554
super(
5655
LOOKUP_ACTION_NAME,
5756
clusterService,
58-
searchService,
57+
createShardContext,
5958
transportService,
6059
bigArrays,
6160
blockFactory,

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlQueryAction.java

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import org.elasticsearch.xpack.esql.action.EsqlQueryTask;
4545
import org.elasticsearch.xpack.esql.core.async.AsyncTaskManagementService;
4646
import org.elasticsearch.xpack.esql.core.expression.FoldContext;
47+
import org.elasticsearch.xpack.esql.enrich.AbstractLookupService;
4748
import org.elasticsearch.xpack.esql.enrich.EnrichLookupService;
4849
import org.elasticsearch.xpack.esql.enrich.EnrichPolicyResolver;
4950
import org.elasticsearch.xpack.esql.enrich.LookupFromIndexService;
@@ -107,8 +108,23 @@ public TransportEsqlQueryAction(
107108
exchangeService.registerTransportHandler(transportService);
108109
this.exchangeService = exchangeService;
109110
this.enrichPolicyResolver = new EnrichPolicyResolver(clusterService, transportService, planExecutor.indexResolver());
110-
this.enrichLookupService = new EnrichLookupService(clusterService, searchService, transportService, bigArrays, blockFactory);
111-
this.lookupFromIndexService = new LookupFromIndexService(clusterService, searchService, transportService, bigArrays, blockFactory);
111+
AbstractLookupService.CreateShardContext lookupCreateShardContext = AbstractLookupService.CreateShardContext.fromSearchService(
112+
searchService
113+
);
114+
this.enrichLookupService = new EnrichLookupService(
115+
clusterService,
116+
lookupCreateShardContext,
117+
transportService,
118+
bigArrays,
119+
blockFactory
120+
);
121+
this.lookupFromIndexService = new LookupFromIndexService(
122+
clusterService,
123+
lookupCreateShardContext,
124+
transportService,
125+
bigArrays,
126+
blockFactory
127+
);
112128
this.computeService = new ComputeService(
113129
searchService,
114130
transportService,

0 commit comments

Comments
 (0)