-
Notifications
You must be signed in to change notification settings - Fork 123
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: update max_in_use_session at 10 mins interval #3570
fix: update max_in_use_session at 10 mins interval #3570
Conversation
cce4419
to
2eec9a6
Compare
2eec9a6
to
ab9f3c5
Compare
@@ -2048,6 +2048,7 @@ boolean isClosed() { | |||
|
|||
// Does various pool maintenance activities. | |||
void maintainPool() { | |||
Instant currTime = clock.instant(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Do we really have to initialize here? Can we initialize before if statement inside synchronized block so that we will get exact current time. It would be okay to initialize here if we don't have synchronized block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed that go does the same way where they are assigning right before if statement. https://github.com/googleapis/google-cloud-go/blob/main/spanner/session.go#L1775
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this currTime
is used at multiple places. Inside synchronized block and outside as well. It will be an overkill if we initialize at multiple times. Inside block and outside. WDYT ?
cc1753d
into
googleapis:main
This PR fixes the max_in_use_session value. Currently this value is only reset at client initialization.
This PR fixes the max_in_use_session value. Currently this value is only reset at client initialization.