Skip to content
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

Ensure ExchangeClient is always properly closed #10950

Merged
merged 1 commit into from
Feb 10, 2022

Conversation

arhimondr
Copy link
Contributor

@arhimondr arhimondr commented Feb 4, 2022

Description

The exchange client used to pull query results may not get closed if a
query is abandoned or even when query finished successfully.

General information

This is a very old problem that didn't manifest itself until #10507. Now since the ExchangeClient may create files when spooling it is required for the temporary data to be properly disposed upon query completion.

Related issues, pull requests, and links

#10507

Documentation

No documentation is needed.

Release notes

No release notes entries required.

@cla-bot cla-bot bot added the cla-signed label Feb 4, 2022
@arhimondr arhimondr force-pushed the close-exchange-client branch 8 times, most recently from 206c61e to 821761d Compare February 7, 2022 15:34
@@ -450,6 +450,8 @@ private synchronized QueryResults getNextResult(long token, UriInfo uriInfo, Dat
}
else {
nextToken = OptionalLong.empty();
// the client is not coming back, make sure the exchangeClient is closed
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be handled above by closeExchangeClientIfNecessary()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

closeExchangeClientIfNecessary only closes the client if a query fails or if a query returns no results (e.g.: a DDL query)

@arhimondr arhimondr force-pushed the close-exchange-client branch 14 times, most recently from c4afc59 to 98507e9 Compare February 8, 2022 21:36
@arhimondr arhimondr changed the title Close exchange client Ensure ExchangeClient is always properly closed Feb 8, 2022
The exchange client used to pull query results may not get closed if a
query is abandoned or even when query finished successfully.
@arhimondr arhimondr force-pushed the close-exchange-client branch from 98507e9 to b2fbe05 Compare February 8, 2022 21:37
@arhimondr arhimondr marked this pull request as ready for review February 8, 2022 21:41
@arhimondr arhimondr requested a review from losipiuk February 8, 2022 21:41
@losipiuk losipiuk merged commit 81212b0 into trinodb:master Feb 10, 2022
@arhimondr arhimondr deleted the close-exchange-client branch February 10, 2022 15:33
@github-actions github-actions bot added this to the 371 milestone Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants