Skip to content

Closing RestRepository to avoid a connection leak #1781

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 25, 2021

Conversation

masseyke
Copy link
Member

This commit closes the RestRepository that is sent to the ScrollQuery constructor to avoid leaking connections to
Elasticsearch when many fast scrolls are called in a tight loop.
Closes #1212

@masseyke
Copy link
Member Author

It seems a little wrong to have ScrollQuery closing an object that it did not open. But it is only used in this one place, and it is appropriate for the RestRepository to be closed in this case. So it's probably not worth doing something more complex (like adding the machinery to have a listener called from ScrollQuery's close).

Copy link
Member

@jbaiera jbaiera left a comment

Choose a reason for hiding this comment

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

LGTM, nice work!

@masseyke masseyke merged commit a7f04d9 into elastic:master Oct 25, 2021
@masseyke masseyke deleted the fix/scroll-connection-leak branch October 25, 2021 20:52
masseyke added a commit to masseyke/elasticsearch-hadoop that referenced this pull request Oct 25, 2021
This commit closes the RestRepository that is sent to the ScrollQuery constructor to avoid leaking connections to
Elasticsearch when many fast scrolls are called in a tight loop.
Closes elastic#1212
masseyke added a commit that referenced this pull request Oct 25, 2021
This commit closes the RestRepository that is sent to the ScrollQuery constructor to avoid leaking connections to
Elasticsearch when many fast scrolls are called in a tight loop.
Relates #1212 #1781
@masseyke masseyke added the bug label Dec 3, 2021
@jrodewig jrodewig added the :Rest label Dec 3, 2021
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.

Connection leak
4 participants