Smarter field caps with subscribable listener#116755
Conversation
…smarter_fieldcaps_calls
…smarter_field_caps_subscribableListener
|
@elasticmachine run elasticsearch-ci/part-2 |
|
@elasticmachine update branch |
…smarter_field_caps_subscribableListener
…hub.com/astefan/elasticsearch into smarter_field_caps_subscribableListener
…smarter_field_caps_subscribableListener
|
@dnhatn @original-brownbear asking for a double-checking of how I tried to use the SubscribableListener. |
…smarter_field_caps_subscribableListener
…smarter_field_caps_subscribableListener
…smarter_field_caps_subscribableListener
|
@elasticmachine update branch |
| class IndexResolutionWrappingListener implements ActionListener<EnrichResolution> { | ||
|
|
||
| private final ActionListener<Tuple<EnrichResolution, IndexResolution>> delegate; | ||
| private final IndexResolution indexResolution; | ||
|
|
||
| IndexResolutionWrappingListener( | ||
| ActionListener<Tuple<EnrichResolution, IndexResolution>> delegate, | ||
| IndexResolution indexResolution | ||
| ) { | ||
| this.delegate = delegate; | ||
| this.indexResolution = indexResolution; | ||
| } | ||
|
|
||
| @Override | ||
| public void onResponse(EnrichResolution enrichResolution) { | ||
| delegate.onResponse(new Tuple<>(enrichResolution, indexResolution)); | ||
| } | ||
|
|
||
| @Override | ||
| public void onFailure(Exception e) { | ||
| delegate.onFailure(e); | ||
| } | ||
| } | ||
|
|
||
| class EnrichResolutionWrappingListener implements ActionListener<IndexResolution> { | ||
|
|
There was a problem hiding this comment.
These two classes are very similar - use a base class with Generics to pass the parameter in.
| // whatever tuple we have here (from CCS-special handling or from the original pre-analysis), pass it on to the next step | ||
| l.onResponse(tuple); | ||
| }) | ||
| .<Tuple<EnrichResolution, IndexResolution>>andThen((l, tuple) -> { |
There was a problem hiding this comment.
👍
This style is more readable.
Furthermore I would extract the lambda in each stage in a separate method - it improves readability and avoids casts (inferred based on the method signature).
| QueryBuilder requestFilter, | ||
| ListenerResult listenerResult, | ||
| ActionListener<ListenerResult> l, | ||
| ActionListener<LogicalPlan> logicalPlanListener |
There was a problem hiding this comment.
Passing and completing the logicalPlanListener within this method does not seem to integrate well into the chain, as it causes the chain to abruptly terminate. Ideally, the SubscribableListener should execute sequentially and asynchronously, block by block, until it encounters a failure. Could we implement retries within this block by calling the method again without a filter?
…smarter_field_caps_subscribableListener
…hub.com/astefan/elasticsearch into smarter_field_caps_subscribableListener
|
@elasticmachine run elasticsearch-ci/part-2 |
|
@elasticmachine update branch |
astefan
added
the
auto-backport
💔 Backport failed
You can use sqren/backport to manually backport by running |
(cherry picked from commit 22f4a79)
* Add CCS tests for index filtering See also: #116755
* Add CCS tests for index filtering See also: elastic#116755
7 participants
This change is two fold:
EsqlSessionto use (hopefully) a more readable and manageable series of async calls that start with pre-analysis of enrich policies, regular indices, lookup indices and end with verification of the analyzed logical plan of the query. It's changing the nested calls using ActionListeners to a "flat" series of steps usingSubscribableListenerVerificationExceptionbeing thrown in the verification step (following the analysis of the logical plan) and given a REST request filter was used, it removes the filter and retries the analysis. Assuming the first call toIndexResolverusing the filter (and implicitly to_field_capscall where the filter is being used, as well) fails, it's retries the_field_capscall without a filter.There is a leftover task (to be addressed in a separate PR) that should add tests in CCS scenarios: #118054
Addresses #115053