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

fix(spanner): delete idle sessions upon SessionPool destruction #14012

Merged
merged 1 commit into from
Apr 17, 2024

Conversation

devbww
Copy link
Contributor

@devbww devbww commented Apr 17, 2024

Send a fire-and-forget AsyncDeleteSession() request for all idle sessions during ~SessionPool(). Previously they would have continued to exist in the service until garbage collected due to inactivity after 60 minutes.

Add expectations for these requests in the connection/session_pool unit tests.


This change is Reviewable

Send a fire-and-forget `AsyncDeleteSession()` request for all idle
sessions during `~SessionPool()`.  Previously they would have continued
to exist in the service until garbage collected due to inactivity
after 60 minutes.

Add expectations for these requests in the connection/session_pool
unit tests.
@product-auto-label product-auto-label bot added the api: spanner Issues related to the Spanner API. label Apr 17, 2024
@devbww devbww marked this pull request as ready for review April 17, 2024 17:13
@devbww devbww requested a review from a team as a code owner April 17, 2024 17:13
for (auto const& session : sessions_) {
if (session->is_bad()) continue;
AsyncDeleteSession(cq_, session->channel()->stub, session->session_name())
.then([](auto result) { auto status = result.get(); });
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the .then() really needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I don't believe so, but I thought it made things a little clearer, particularly if we later to chose to do any logging/tracing.

@devbww devbww merged commit c9e40d9 into googleapis:main Apr 17, 2024
62 checks passed
@devbww devbww deleted the delete-sessions branch April 17, 2024 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the Spanner API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants