Fix missing index exception handling #126738

smalyshev · 2025-04-12T00:20:33Z

This fixes the issue with missing indices being ignored (see #126275).

I am not in love with this because it looks like a hack, and proper fix would be to move handling of this into verifier on the planning stage, but the way field caps works now it doesn't seem possible without protocol changes.

Also, it doesn't fix the case of

FROM transform-es-search-*,missing | LIMIT 0

which now seems to be broken regardless of partials being enabled (see #114495)

I think this patch makes sense in any case as from what I understand we never want to ignore missing concrete index, but with proper index resolution fix the only way we should ever encounter it would be if the index is gone between planning and execution stage.

Copilot

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

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

[nitpick] Consider using the negation operator instead of '== false' (i.e. '&& !(ExceptionsHelper.unwrapCause(e) instanceof IndexNotFoundException)') to improve readability.

&& (ExceptionsHelper.unwrapCause(e) instanceof IndexNotFoundException) == false) {

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/ClusterComputeHandler.java:94

[nitpick] Consider replacing '== false' with a negation operator (i.e. '&& !(ExceptionsHelper.unwrapCause(e) instanceof IndexNotFoundException)') to enhance clarity.

&& (ExceptionsHelper.unwrapCause(e) instanceof IndexNotFoundException) == false) {

elasticsearchmachine · 2025-04-12T00:23:31Z

Hi @smalyshev, I've created a changelog YAML for you.

… missing indices So we have to drop FILTERED status for now

elasticsearchmachine · 2025-04-23T15:32:32Z

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

elasticsearchmachine · 2025-04-23T15:32:33Z

Pinging @elastic/es-search-foundations (Team:Search Foundations)

quux00

LGTM

smalyshev · 2025-04-23T17:47:30Z

...ternalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClusterUsageTelemetryIT.java

@@ -138,32 +138,6 @@ public void testFailed() throws Exception {
        assertThat(telemetry.getByRemoteCluster().size(), equalTo(0));
    }

-    // TODO: enable when skip-un patch is merged


This is just old test we never enabled and we don't really need it anymore, so cleaning it up.

dnhatn · 2025-04-23T22:34:13Z

Thanks @smalyshev.

I took a look at the issue and I think there are several problems:

There is a disparity in the indices option between the field-caps API (planning time) and the search-shards API (runtime). We use ALLOW_UNAVAILABLE_TARGETS for field-caps and ERROR_WHEN_UNAVAILABLE_TARGETS for search-shards. This leads to cases where field-caps does not return failures, but the runtime does. With allow_partial_results, we then ignore the runtime failures and return partial results instead of failing the request.
We do not strictly check the index failures returned by the field-caps API.
Another issue is related to security exceptions. Since we use ALLOW_UNAVAILABLE_TARGETS in the field-caps API, it returns unknown index if users lack the privilege to access it. However, if multiple index patterns are specified, we return an unauthorized error from the runtime instead (see EsqlSecurityIT).
There are cases where we return a 400 error, and others where we return a 404.

I wonder if we should spend more time addressing these inconsistencies rather than fixing the surface issues. I am concerned that more surface fixes could accumulate over time. (@astefan WDYT?)

I can give it a try to address these.

smalyshev · 2025-04-24T00:24:49Z

@dnhatn You are right, to make a proper and comprehensive fix we need to change how we call field-caps, and maybe to change field-caps API. This patch is not that fix, it's just a quick plug to unblock progress with partial results. I agree that we should work on fixing this, I am just not sure how long it could take, so if partial results work remains blocked until we finish fixing that, and it takes long, we may have problem wrapping it up in time for 8.19. But if you can think about a fix that we can implement reasonably quickly then it's of course better.

That said, I think even after that fix, if we get IndexNotFound in runtime, we probably need to produce an error. I can't see any scenario where we get a missing index and don't want an error?

The discrepancy between 400 and 404 is because VerificationError is 400, and runtime not-found error is 404. If we alawys want 404 on unknown index, we'd need to make verification error more flexible, probably.

dnhatn · 2025-04-24T01:14:15Z

@smalyshev I agree. Let's try this until the end of the week. If we can't find a proper fix, we should proceed with this to unblock the partial results.

dnhatn

Thinking about this more, I am okay with getting this in now. We can make the proper changes later. Thanks @smalyshev

elasticsearchmachine · 2025-04-24T04:28:13Z

💔 Backport failed

Status	Branch	Result
❌	8.x	Commit could not be cherrypicked due to conflicts

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

smalyshev · 2025-04-24T04:29:36Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

Questions ?

Please refer to the Backport tool documentation

* Fix missing index handling for partial-enabled queries (cherry picked from commit 54ef165) # Conflicts: # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/ClusterComputeHandler.java

astefan · 2025-04-24T06:38:58Z

Thanks @smalyshev.

I took a look at the issue and I think there are several problems:

There is a disparity in the indices option between the field-caps API (planning time) and the search-shards API (runtime). We use ALLOW_UNAVAILABLE_TARGETS for field-caps and ERROR_WHEN_UNAVAILABLE_TARGETS for search-shards. This leads to cases where field-caps does not return failures, but the runtime does. With allow_partial_results, we then ignore the runtime failures and return partial results instead of failing the request.

We do not strictly check the index failures returned by the field-caps API.

Another issue is related to security exceptions. Since we use ALLOW_UNAVAILABLE_TARGETS in the field-caps API, it returns unknown index if users lack the privilege to access it. However, if multiple index patterns are specified, we return an unauthorized error from the runtime instead (see EsqlSecurityIT).

There are cases where we return a 400 error, and others where we return a 404.

I wonder if we should spend more time addressing these inconsistencies rather than fixing the surface issues. I am concerned that more surface fixes could accumulate over time. (@astefan WDYT?)

I can give it a try to address these.

@dnhatn I agree. The difference between field_caps and _search time behavior has surfaced previously in one of your earlier PRs, but with partial results the impact is more severe. I agree that we need to have a clean look at the way we use ignore_unvailable in these two cases; it is likely this was a leftover from ES SQL where ignore_unavailable=true for field_caps might have had a very specific use (one that is, so far, not needed in ES|QL).

smalyshev · 2025-04-24T16:31:53Z

@dnhatn @astefan I tried to switch the ignore_unavailable here: #126737 to see what happens but it causes a ton of test failures, some of them to do with authorization, so I wasn't able to triage them properly yet.

smalyshev and others added 7 commits April 9, 2025 14:20

Fix missing index handling for partial-enabled queries

436a831

Merge branch 'main' into fix-missing-index-handling

0bacef5

Oops shouldn't delete that

39141cc

Merge branch 'main' into fix-missing-index-handling

5ffdc46

Set ignore_unavaliable to false

d3421a1

Merge branch 'main' into fix-missing-index-handling

374df05

Revert

06ee627

elasticsearchmachine added the v9.1.0 label Apr 12, 2025

smalyshev added :Analytics/ES|QL AKA ESQL >bug v8.19.0 auto-backport Automatically create backport pull requests when merged labels Apr 12, 2025

smalyshev requested a review from Copilot April 12, 2025 00:23

Copilot AI reviewed Apr 12, 2025

View reviewed changes

Update docs/changelog/126738.yaml

bb890ec

smalyshev changed the title ~~Fix missing index exception~~ Fix missing index exception handling Apr 12, 2025

smalyshev and others added 13 commits April 11, 2025 21:30

Merge branch 'main' into fix-missing-index-exception

88602d4

Merge branch 'main' into fix-missing-index-exception

954f753

Merge branch 'main' into fix-missing-index-exception

be41b3e

Merge branch 'main' into fix-missing-index-exception

c2159d2

Test for CCS with filters

9f05ded

[CI] Auto commit changes from spotless

6f348b9

Partial fix for CCS/filters problems

7313daf

fix tests

fb7fb6a

Merge branch 'main' into fix-filter-ccs

8610845

More tests

a51bf2a

We can not eliminate some filtered runtime calls since there could be…

d58d6e6

… missing indices So we have to drop FILTERED status for now

Merge branch 'main' into fix-filter-ccs

9f432eb

Merge branch 'main' into fix-filter-ccs

c730f21

smalyshev marked this pull request as ready for review April 23, 2025 15:32

smalyshev requested a review from quux00 April 23, 2025 15:32

elasticsearchmachine added Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch labels Apr 23, 2025

smalyshev requested a review from dnhatn April 23, 2025 15:32

Fix merge

a8e3efb

quux00 approved these changes Apr 23, 2025

View reviewed changes

smalyshev enabled auto-merge (squash) April 23, 2025 17:33

smalyshev disabled auto-merge April 23, 2025 17:33

oops merge dropped this change, restore

44e559a

smalyshev enabled auto-merge (squash) April 23, 2025 17:40

smalyshev disabled auto-merge April 23, 2025 17:43

smalyshev commented Apr 23, 2025

View reviewed changes

dnhatn approved these changes Apr 24, 2025

View reviewed changes

smalyshev merged commit 54ef165 into elastic:main Apr 24, 2025
17 checks passed

elasticsearchmachine added the backport pending label Apr 24, 2025

smalyshev mentioned this pull request Apr 24, 2025

[8.x] Fix missing index exception handling (#126738) #127301

Merged

smalyshev removed the backport pending label Apr 24, 2025

smalyshev mentioned this pull request Apr 25, 2025

Allow partial results by default in ES|QL - Take 2 #127351

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix missing index exception handling #126738

Fix missing index exception handling #126738

smalyshev commented Apr 12, 2025 •

edited

Loading

Copilot AI left a comment

elasticsearchmachine commented Apr 12, 2025

elasticsearchmachine commented Apr 23, 2025

elasticsearchmachine commented Apr 23, 2025

quux00 left a comment

smalyshev Apr 23, 2025

dnhatn commented Apr 23, 2025

smalyshev commented Apr 24, 2025

dnhatn commented Apr 24, 2025

dnhatn left a comment

elasticsearchmachine commented Apr 24, 2025

smalyshev commented Apr 24, 2025

astefan commented Apr 24, 2025

smalyshev commented Apr 24, 2025

Fix missing index exception handling #126738

Fix missing index exception handling #126738

Conversation

smalyshev commented Apr 12, 2025 • edited Loading

Copilot AI left a comment

Choose a reason for hiding this comment

elasticsearchmachine commented Apr 12, 2025

elasticsearchmachine commented Apr 23, 2025

elasticsearchmachine commented Apr 23, 2025

quux00 left a comment

Choose a reason for hiding this comment

smalyshev Apr 23, 2025

Choose a reason for hiding this comment

dnhatn commented Apr 23, 2025

smalyshev commented Apr 24, 2025

dnhatn commented Apr 24, 2025

dnhatn left a comment

Choose a reason for hiding this comment

elasticsearchmachine commented Apr 24, 2025

💔 Backport failed

smalyshev commented Apr 24, 2025

💚 All backports created successfully

Questions ?

astefan commented Apr 24, 2025

smalyshev commented Apr 24, 2025

smalyshev commented Apr 12, 2025 •

edited

Loading