Skip to content

Commit 7539fb7

Browse files
authored
ESQL: Make it a little easier to test LOOKUP (#120540) (#120555)
* 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 c5a623a commit 7539fb7

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
@@ -129,10 +129,10 @@
129129
* the same number of rows that it was sent no matter how many documents match.
130130
* </p>
131131
*/
132-
abstract class AbstractLookupService<R extends AbstractLookupService.Request, T extends AbstractLookupService.TransportRequest> {
132+
public abstract class AbstractLookupService<R extends AbstractLookupService.Request, T extends AbstractLookupService.TransportRequest> {
133133
private final String actionName;
134134
private final ClusterService clusterService;
135-
private final SearchService searchService;
135+
private final CreateShardContext createShardContext;
136136
private final TransportService transportService;
137137
private final Executor executor;
138138
private final BigArrays bigArrays;
@@ -151,7 +151,7 @@ abstract class AbstractLookupService<R extends AbstractLookupService.Request, T
151151
AbstractLookupService(
152152
String actionName,
153153
ClusterService clusterService,
154-
SearchService searchService,
154+
CreateShardContext createShardContext,
155155
TransportService transportService,
156156
BigArrays bigArrays,
157157
BlockFactory blockFactory,
@@ -160,7 +160,7 @@ abstract class AbstractLookupService<R extends AbstractLookupService.Request, T
160160
) {
161161
this.actionName = actionName;
162162
this.clusterService = clusterService;
163-
this.searchService = searchService;
163+
this.createShardContext = createShardContext;
164164
this.transportService = transportService;
165165
this.executor = transportService.getThreadPool().executor(ThreadPool.Names.SEARCH);
166166
this.bigArrays = bigArrays;
@@ -326,9 +326,8 @@ private void doLookup(T request, CancellableTask task, ActionListener<List<Page>
326326
final List<Releasable> releasables = new ArrayList<>(6);
327327
boolean started = false;
328328
try {
329-
final ShardSearchRequest shardSearchRequest = new ShardSearchRequest(request.shardId, 0, AliasFilter.EMPTY);
330-
final SearchContext searchContext = searchService.createSearchContext(shardSearchRequest, SearchService.NO_TIMEOUT);
331-
releasables.add(searchContext);
329+
LookupShardContext shardContext = createShardContext.create(request.shardId);
330+
releasables.add(shardContext.release);
332331
final LocalCircuitBreaker localBreaker = new LocalCircuitBreaker(
333332
blockFactory.breaker(),
334333
localBreakerSettings.overReservedBytes(),
@@ -366,8 +365,7 @@ private void doLookup(T request, CancellableTask task, ActionListener<List<Page>
366365
}
367366
}
368367
releasables.add(finishPages);
369-
SearchExecutionContext searchExecutionContext = searchContext.getSearchExecutionContext();
370-
QueryList queryList = queryList(request, searchExecutionContext, inputBlock, request.inputDataType);
368+
QueryList queryList = queryList(request, shardContext.executionContext, inputBlock, request.inputDataType);
371369
var warnings = Warnings.createWarnings(
372370
DriverContext.WarningsMode.COLLECT,
373371
request.source.source().getLineNumber(),
@@ -378,11 +376,11 @@ private void doLookup(T request, CancellableTask task, ActionListener<List<Page>
378376
driverContext.blockFactory(),
379377
EnrichQuerySourceOperator.DEFAULT_MAX_PAGE_SIZE,
380378
queryList,
381-
searchExecutionContext.getIndexReader(),
379+
shardContext.context.searcher().getIndexReader(),
382380
warnings
383381
);
384382
releasables.add(queryOperator);
385-
var extractFieldsOperator = extractFieldsOperator(searchContext, driverContext, request.extractFields);
383+
var extractFieldsOperator = extractFieldsOperator(shardContext.context, driverContext, request.extractFields);
386384
releasables.add(extractFieldsOperator);
387385

388386
/*
@@ -405,7 +403,7 @@ private void doLookup(T request, CancellableTask task, ActionListener<List<Page>
405403
List.of(extractFieldsOperator, finishPages),
406404
outputOperator,
407405
Driver.DEFAULT_STATUS_INTERVAL,
408-
Releasables.wrap(searchContext, localBreaker)
406+
Releasables.wrap(shardContext.release, localBreaker)
409407
);
410408
task.addListener(() -> {
411409
String reason = Objects.requireNonNullElse(task.getReasonCancelled(), "task was cancelled");
@@ -430,15 +428,10 @@ private void doLookup(T request, CancellableTask task, ActionListener<List<Page>
430428
}
431429

432430
private static Operator extractFieldsOperator(
433-
SearchContext searchContext,
431+
EsPhysicalOperationProviders.ShardContext shardContext,
434432
DriverContext driverContext,
435433
List<NamedExpression> extractFields
436434
) {
437-
EsPhysicalOperationProviders.ShardContext shardContext = new EsPhysicalOperationProviders.DefaultShardContext(
438-
0,
439-
searchContext.getSearchExecutionContext(),
440-
searchContext.request().getAliasFilter()
441-
);
442435
List<ValuesSourceReaderOperator.FieldInfo> fields = new ArrayList<>(extractFields.size());
443436
for (NamedExpression extractField : extractFields) {
444437
BlockLoader loader = shardContext.blockLoader(
@@ -462,7 +455,7 @@ private static Operator extractFieldsOperator(
462455
return new ValuesSourceReaderOperator(
463456
driverContext.blockFactory(),
464457
fields,
465-
List.of(new ValuesSourceReaderOperator.ShardContext(searchContext.searcher().getIndexReader(), searchContext::newSourceLoader)),
458+
List.of(new ValuesSourceReaderOperator.ShardContext(shardContext.searcher().getIndexReader(), shardContext::newSourceLoader)),
466459
0
467460
);
468461
}
@@ -670,4 +663,42 @@ public boolean hasReferences() {
670663
return refs.hasReferences();
671664
}
672665
}
666+
667+
/**
668+
* Create a {@link LookupShardContext} for a locally allocated {@link ShardId}.
669+
*/
670+
public interface CreateShardContext {
671+
LookupShardContext create(ShardId shardId) throws IOException;
672+
673+
static CreateShardContext fromSearchService(SearchService searchService) {
674+
return shardId -> {
675+
ShardSearchRequest shardSearchRequest = new ShardSearchRequest(shardId, 0, AliasFilter.EMPTY);
676+
return LookupShardContext.fromSearchContext(
677+
searchService.createSearchContext(shardSearchRequest, SearchService.NO_TIMEOUT)
678+
);
679+
};
680+
}
681+
}
682+
683+
/**
684+
* {@link AbstractLookupService} uses this to power the queries and field loading that
685+
* it needs to perform to actually do the lookup.
686+
*/
687+
public record LookupShardContext(
688+
EsPhysicalOperationProviders.ShardContext context,
689+
SearchExecutionContext executionContext,
690+
Releasable release
691+
) {
692+
public static LookupShardContext fromSearchContext(SearchContext context) {
693+
return new LookupShardContext(
694+
new EsPhysicalOperationProviders.DefaultShardContext(
695+
0,
696+
context.getSearchExecutionContext(),
697+
context.request().getAliasFilter()
698+
),
699+
context.getSearchExecutionContext(),
700+
context
701+
);
702+
}
703+
}
673704
}

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)