Handle failures that occur during field-caps #130840

dnhatn · 2025-07-08T18:40:23Z

Currently, errors from the field-caps phase are not always handled properly, leading to cases where the final response is not marked as partial correctly. For example: FROM ok*,unavailable_index* should return a partial result, as unavailable_index* is skipped after the resolution phase. This change tracks failures that occur during field-caps and reports them in the final response. Since this only affects cases with allow_partial_results=true, I am labeling this as a non-issue and will backport the change to 9.1 and 8.19.

elasticsearchmachine · 2025-07-09T17:48:22Z

Pinging @elastic/es-analytical-engine (Team:Analytics)

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

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

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

smalyshev · 2025-07-09T19:41:37Z

...sterTest/java/org/elasticsearch/xpack/esql/action/CrossClusterQueryWithPartialResultsIT.java

        }
    }

+    public void testResolutionFailures() throws Exception {


Check out also #129957 - I've made some tests for things that weren't handled properly.

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

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

...sterTest/java/org/elasticsearch/xpack/esql/action/CrossClusterQueryWithPartialResultsIT.java

dnhatn · 2025-07-10T17:43:14Z

@smalyshev @idegtiarenko Thanks for review. I think it's ready again.

...sterTest/java/org/elasticsearch/xpack/esql/action/CrossClusterQueryWithPartialResultsIT.java

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

smalyshev · 2025-07-11T17:26:26Z

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

    ) {
-        Set<String> clustersWithResolvedIndices = new HashSet<>();
-        // determine missing clusters
+        final Set<String> clustersWithNoMatchingIndices = new HashSet<>(executionInfo.clusterAliases());


I still think we should use something like executionInfo.getClusterStates(RUNNING) here. That should automatically exclude unavailable clusters if we call updateExecutionInfoWithUnavailableClusters before this. I think this would also let us eliminate unavailableClusters parameter.

Updated in 77aacf2

I had to revert this refactoring because the tests in EsqlCCSUtilsTests assume these conditions together. Unfortunately, I don't have the bandwidth to work on this now. Could you work on this after this PR? Thanks!

smalyshev · 2025-07-11T17:28:56Z

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

+                        // when queries use a remote cluster wildcard, e.g., `*:my-logs*`.
+                        Exception nonIndexNotFound = failures.stream()
+                            .map(FieldCapabilitiesFailure::getException)
+                            .filter(ex -> ExceptionsHelper.unwrap(ex, IndexNotFoundException.class) == null)


Question here: are all situations where we can't use index for some reason (security, closed, hidden, whatever it is) would be covered by IndexNotFoundException? If not, we still could get unhelpful failures here.

We should report all errors, including IndexNotFoundException. This special handling exists because index_options are not available in ES|QL; otherwise, we shouldn't have it.

smalyshev · 2025-07-11T17:39:42Z

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

-                EsqlCCSUtils.updateExecutionInfoWithClustersWithNoMatchingIndices(executionInfo, result.indices, null);
+                var unavailableClusters = EsqlCCSUtils.determineUnavailableRemoteClusters(result.indices.failures());
+                EsqlCCSUtils.updateExecutionInfoWithUnavailableClusters(executionInfo, unavailableClusters);
+                EsqlCCSUtils.updateExecutionInfoWithClustersWithNoMatchingIndices(


I feel like it may be better to move updateExecutionInfoWithUnavailableClusters into updateExecutionInfoWithUnavailableClusters and have updateExecutionInfoWithClustersWithNoMatchingIndices use cluster statuses (since unavailables would get marked as skipped). What do you think?

Yeah, it's a good idea, but I don’t have bandwidth for it right now and would prefer to focus on getting this in for 8.19/9.1 as soon as possible. Would you be able to work on it later?

Sure I could make a followup on this.

👍 Thank you!

This reverts commit 77aacf2.

smalyshev · 2025-07-14T18:25:32Z

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

-                EsqlCCSUtils.updateExecutionInfoWithClustersWithNoMatchingIndices(executionInfo, result.indices, null);
+                var unavailableClusters = EsqlCCSUtils.determineUnavailableRemoteClusters(result.indices.failures());
+                EsqlCCSUtils.updateExecutionInfoWithUnavailableClusters(executionInfo, unavailableClusters);
+                EsqlCCSUtils.updateExecutionInfoWithClustersWithNoMatchingIndices(


Sure I could make a followup on this.

dnhatn · 2025-07-14T22:15:51Z

@smalyshev @idegtiarenko Thank you for reviews!

elasticsearchmachine · 2025-07-14T22:17:36Z

💔 Backport failed

Status	Branch	Result
❌	9.1	Commit could not be cherrypicked due to conflicts
❌	8.19	Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 130840

Currently, errors from the field-caps phase are not always handled properly, leading to cases where the final response is not marked as partial correctly. For example: FROM ok*,unavailable_index* should return a partial result, as unavailable_index* is skipped after the resolution phase. This change tracks failures that occur during field-caps and reports them in the final response. Since this only affects cases with allow_partial_results=true, I am labeling this as a non-issue and will backport the change to 9.1 and 8.19.

Currently, errors from the field-caps phase are not always handled properly, leading to cases where the final response is not marked as partial correctly. For example: FROM ok*,unavailable_index* should return a partial result, as unavailable_index* is skipped after the resolution phase. This change tracks failures that occur during field-caps and reports them in the final response. Since this only affects cases with allow_partial_results=true, I am labeling this as a non-issue and will backport the change to 9.1 and 8.19. (cherry picked from commit a699655)

dnhatn · 2025-07-15T01:16:04Z

💚 All backports created successfully

Status	Branch	Result
✅	9.1
✅	8.19

Questions ?

Please refer to the Backport tool documentation

Currently, errors from the field-caps phase are not always handled properly, leading to cases where the final response is not marked as partial correctly. For example: FROM ok*,unavailable_index* should return a partial result, as unavailable_index* is skipped after the resolution phase. This change tracks failures that occur during field-caps and reports them in the final response. Since this only affects cases with allow_partial_results=true, I am labeling this as a non-issue and will backport the change to 9.1 and 8.19. (cherry picked from commit a699655)

Currently, errors from the field-caps phase are not always handled properly, leading to cases where the final response is not marked as partial correctly. For example: FROM ok*,unavailable_index* should return a partial result, as unavailable_index* is skipped after the resolution phase. This change tracks failures that occur during field-caps and reports them in the final response. Since this only affects cases with allow_partial_results=true, I am labeling this as a non-issue and will backport the change to 9.1 and 8.19.

elasticsearchmachine added the v9.2.0 label Jul 8, 2025

dnhatn force-pushed the resolution-errors branch 3 times, most recently from a74566a to 8fede66 Compare July 9, 2025 05:39

dnhatn added :Analytics/ES|QL AKA ESQL >non-issue labels Jul 9, 2025

Handle failures that occur during field-caps

d7f257d

dnhatn force-pushed the resolution-errors branch from 8fede66 to d7f257d Compare July 9, 2025 16:14

dnhatn added v9.1.1 v8.19.1 labels Jul 9, 2025

dnhatn requested review from idegtiarenko, quux00 and smalyshev July 9, 2025 17:46

dnhatn added the auto-backport Automatically create backport pull requests when merged label Jul 9, 2025

dnhatn marked this pull request as ready for review July 9, 2025 17:47

elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Jul 9, 2025

smalyshev reviewed Jul 9, 2025

View reviewed changes

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlExecutionInfo.java Outdated Show resolved Hide resolved

smalyshev reviewed Jul 9, 2025

View reviewed changes

dnhatn added 3 commits July 9, 2025 21:51

Restore isPartial

a1d64a8

restore no matching

3eedd02

Merge remote-tracking branch 'elastic/main' into resolution-errors

352ce2b

idegtiarenko reviewed Jul 10, 2025

View reviewed changes

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/index/IndexResolution.java Show resolved Hide resolved

idegtiarenko reviewed Jul 10, 2025

View reviewed changes

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java Outdated Show resolved Hide resolved

idegtiarenko reviewed Jul 10, 2025

View reviewed changes

...sterTest/java/org/elasticsearch/xpack/esql/action/CrossClusterQueryWithPartialResultsIT.java Show resolved Hide resolved

dnhatn added 4 commits July 10, 2025 09:46

Merge remote-tracking branch 'elastic/main' into resolution-errors

67e0fec

Add docs

9b19452

Add target shard

406f8b2

unwrap once

2c1b04f

dnhatn requested a review from smalyshev July 10, 2025 17:42

dnhatn requested a review from idegtiarenko July 10, 2025 17:42

Merge branch 'main' into resolution-errors

723a07f

smalyshev reviewed Jul 11, 2025

View reviewed changes

dnhatn added 4 commits July 13, 2025 17:48

Merge remote-tracking branch 'elastic/main' into resolution-errors

2e18273

More detailed error

23a2f9b

Add comment

586f4b4

no matching indices

77aacf2

dnhatn requested a review from smalyshev July 14, 2025 03:54

dnhatn added 4 commits July 13, 2025 22:16

Revert "no matching indices"

300567b

This reverts commit 77aacf2.

Merge remote-tracking branch 'elastic/main' into resolution-errors

5c5e77e

Merge remote-tracking branch 'elastic/main' into resolution-errors

7aa9f59

Merge remote-tracking branch 'elastic/main' into resolution-errors

57ac10f

smalyshev approved these changes Jul 14, 2025

View reviewed changes

dnhatn merged commit a699655 into elastic:main Jul 14, 2025
33 checks passed

dnhatn deleted the resolution-errors branch July 14, 2025 22:16

elasticsearchmachine added the backport pending label Jul 14, 2025

dnhatn mentioned this pull request Jul 15, 2025

[9.1] Handle failures that occur during field-caps (#130840) #131254

Merged

dnhatn mentioned this pull request Jul 15, 2025

[8.19] Handle failures that occur during field-caps (#130840) #131255

Merged

dnhatn removed the backport pending label Jul 15, 2025

Handle failures that occur during field-caps #130840

Handle failures that occur during field-caps #130840

Uh oh!

Conversation

dnhatn commented Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticsearchmachine commented Jul 9, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dnhatn commented Jul 10, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dnhatn Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dnhatn commented Jul 14, 2025

Uh oh!

Uh oh!

elasticsearchmachine commented Jul 14, 2025

💔 Backport failed

Uh oh!

dnhatn commented Jul 15, 2025

💚 All backports created successfully

Questions ?

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dnhatn commented Jul 8, 2025 •

edited

Loading

dnhatn Jul 14, 2025 •

edited

Loading