Skip to content

rest5client: cancel http request on interruption in the sync api#1197

Merged
l-trotta merged 3 commits intoelastic:mainfrom
lovasoa:rest5client-sync-interrup-propagation
Apr 8, 2026
Merged

rest5client: cancel http request on interruption in the sync api#1197
l-trotta merged 3 commits intoelastic:mainfrom
lovasoa:rest5client-sync-interrup-propagation

Conversation

@lovasoa
Copy link
Copy Markdown
Contributor

@lovasoa lovasoa commented Apr 1, 2026

When a thread running a synchronous Rest5Client request is interrupted, cancel the underlying http request instead of letting it run to completion.

This avoids runaway tasks running on the ES cluster while their client has been interrupted and will never read the results.

fixes #1195

This is an alternative to #1196 . That PR fixed the issue with any http client, including those outside of our control that have the same issue, but required using performRequestAsync under the hood even on http clients that may not have the issue.

This PR focuses on our own Rest5 client and fixes the issue at a lower level. Users of the org.elasticsearch.client.RestClient (which is not defined in this repo) will still be affected by the issue.

When the thread is interrupted, cancel the underlying http request
instead of letting it run to completion.

This avoids runaway tasks running on the ES cluster while their client
has been interrupted and will never read the results.

fixes elastic#1195

This is an alternative to
elastic#1196 .
That pr fixed the issue with any http client, including those outside of
our control that have the same issue.
This PR focuses on our own rest client. The bug will still be present
when the library is used with other http clients that do not cancel
requests on interruption.
@lovasoa
Copy link
Copy Markdown
Contributor Author

lovasoa commented Apr 3, 2026

If someone has time, could you please review this ? @Mpdreamz maybe ?
The change itself is only 4 lines, the rest is just a new test case to prevent regression.

@lovasoa lovasoa changed the title rest5client: cancel http request on interruption rest5client: cancel http request on interruption in the sync api Apr 3, 2026
@Mpdreamz
Copy link
Copy Markdown
Member

Mpdreamz commented Apr 7, 2026

Hey @lovasoa I am no longer on the clients team but I am sure this will be handled soon!

@l-trotta
Copy link
Copy Markdown
Contributor

l-trotta commented Apr 7, 2026

@lovasoa could you make a small change? client.execute() should stay inside the try block, as it was before the changes.

lovasoa added a commit to lovasoa/elasticsearch-java that referenced this pull request Apr 7, 2026
@lovasoa
Copy link
Copy Markdown
Contributor Author

lovasoa commented Apr 7, 2026

@l-trotta I made the change. I think wrapping potential exceptions thrown by client.execute() (as opposed to exceptions thrown when actually running the future) was an error in the first place, but you are right that it's not related to the issue we are fixing here.

Copy link
Copy Markdown
Contributor

@l-trotta l-trotta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you @lovasoa!

@l-trotta l-trotta merged commit de8b85a into elastic:main Apr 8, 2026
8 checks passed
@lovasoa
Copy link
Copy Markdown
Contributor Author

lovasoa commented Apr 8, 2026

Thank you !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

in the sync api, queries are not cancelled when the calling thread is interrupted

3 participants