[ES|QL] Show partial results after timeout

[lukasolson]

Summary

Partially addresses #205783.

When an ES|QL query times out, this PR calls the _query/async/{id}/stop endpoint to retrieve the results of the query instead of simply calling GET (which does not get the completed results).

Updates the fetching in Discover to properly account for partial results in ES|QL.

Release notes

When an ES|QL query times out (as a result of the search:timeout advanced setting), partial results that are available are now shown.

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

• Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
• Documentation was added for features that require explanation or tutorials
• Unit or functional tests were updated or added to match the most common scenarios
• If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
• This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations.
• Flaky Test Runner was used on any tests changed
• The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

Identify risks

Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging.

• See some risk examples
• ...

[stratoula]

@lukasolson could we do the same when the user clicks the Cancel button? (we can do it at a follow up PR too)

[lukasolson]

@lukasolson could we do the same when the user clicks the Cancel button? (we can do it at a follow up PR too)

Yes, this is the plan, but it will be a follow-up PR because we need to make some changes to how the aborting works so we can distinguish between when the user cancels & when we automatically cancel (when refreshing, navigating around, config changes, etc.)

[elasticmachine]

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

[elasticmachine]

Pinging @elastic/kibana-esql (Team:ESQL)

[nreese]

kibana-presentation changes LGTM
code review only

[davismcphee]

Code changes look good and it seems to work well for the most part! Unfortunately I did run into some flakiness where sometimes the results wouldn't show as expected. It seems like there may be a timing issue going on:

timeout.mp4

[davismcphee]

Just curious why the internal user logic is no longer needed?

[lukasolson]

This was actually never used for ES|QL, it was there as a result of a copy & paste of the regular async search strategy. See the discussion here: #174246 (comment)

It's not completely necessary to remove as part of this PR but I was fiddling around in there anyway and decided to remove it since it opens up a backdoor that could potentially cause issues (e.g. an internal engineer uses this feature and exposes an HTTP route that allows any Kibana user to run queries as the internal system user).

[davismcphee]

Thanks for explaining, and in that case totally agree removing it makes sense 👍

[lukasolson]

Unfortunately I did run into some flakiness where sometimes the results wouldn't show as expected. It seems like there may be a timing issue going on:

Hmm interesting, I will look into this.

[lukasolson]

Unfortunately I did run into some flakiness where sometimes the results wouldn't show as expected. It seems like there may be a timing issue going on

That was a pretty wild one to chase down - it appears we were sometimes cancelling the ES|QL request when the HTTP request was aborted (which will happen occasionally depending on exact timing) which was causing this issue. I believe I've fixed it in 9aac375. Do you mind taking another look?

[dmlemeshko]

good catch, tested locally

[stratoula]

This looks great!

I am not sure if this is a win of this PR (I didnt check main) but I see we get a notification on the dashboards too!

[davismcphee]

Nice job tracking that issue down! I left one question, but overall this looks good to go on my end and I can no longer reproduce the flakiness 👍

Just one thing I noticed though. When running the functional test, the query seems to automatically rerun again once it times out, resulting in another search and timeout + toast. Any idea why this might be and if it's an actual issue or just test related? It doesn't seem to happen on subsequent fetches and I wasn't able to reproduce it manually, but we may want to investigate anyway to be sure before merging.

timeout.mp4

[davismcphee]

I'm not really familiar enough with how this code works to know if there are any other implications to this. Will it impact running queries being cancelled when navigating away from Discover?

[lukasolson]

For async queries, we rely on two mechanisms for cancelling queries:

1. The client-side abort logic that will actually send a DELETE {id} request (when a new query is submitted, the "Cancel" button is clicked, navigating between applications in Kibana, etc.)
2. The keep_alive that is updated on every poll (this is an escape hatch for when we can't detect any sort of event that signals that the search request should be terminated, such as when closing the browser completely or navigating outside of Kibana)

The abortSignal that was being passed here fires when the browser disconnects the HTTP request before receiving a response:

What this means is that when the browser cancelled an HTTP request (even just a polling request), the server was then cancelling the query (even though the client would follow up with the DELETE {id} request afterward). This creates a race condition and is not the behavior we want since we need to allow the query to continue running while we make the call to get the final results before we cancel the query.

[lukasolson]

Oh and I forgot to mention that the reason this abortSignal is even around is because, in the case of synchronous queries, we rely on this mechanism to cancel queries since we don't have any sort of ID to call the cancel APIs on.

[davismcphee]

Awesome, thanks for the thorough explanation, much clearer to me now!

[lukasolson]

When running the functional test, the query seems to automatically rerun again once it times out, resulting in another search and timeout + toast. Any idea why this might be and if it's an actual issue or just test related?

Great catch! It was the test. It turns out await timePicker.setDefaultAbsoluteRange(); will actually submit the query without needing to call await testSubjects.click('querySubmitButton'); afterwards. Fixed in f4319f2.

[davismcphee]

Confirmed the double submit issue in the test is now fixed, LGTM 👍 Thanks for working on this!

Also I noticed the backport label was set to backport:prev-minor, which is 9.0, but I think we want it only in 9.1 and 8.19, right? I updated them anyway, but wanted to add a note in case.

[davismcphee]

Awesome, thanks for the thorough explanation, much clearer to me now!

[ppisljar]

LGTM

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: 8a41b9c

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
discover	1.1MB	1.1MB	+192.0B
inspector	27.5KB	27.6KB	+94.0B
total			+286.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
data	402.2KB	402.3KB	+48.0B

History

• 💚 Build #306440 succeeded cb74cd5
• 💔 Build #306431 failed d657ad6
• 💛 Build #305570 was flaky c72ecde
• 💚 Build #305543 succeeded 9aac375
• 💚 Build #299221 succeeded d8d8841
• 💔 Build #299179 failed 281771c

cc @lukasolson

[kibanamachine]

Starting backport for target branches: 8.19

https://github.com/elastic/kibana/actions/runs/15566290770

[kibanamachine]

💚 All backports created successfully

Status	Branch	Result
✅	8.19

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation