feat: optimize maintainer to let sessions be GC'ed instead of deleted #135
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
googlebot
added
the
cla: yes
olavloite
added
the
api: spanner
|
@skuruppu This repo does not need a PR approval before merging. Do we need to change the config? |
|
@olavloite (3) can be reverted because it turns out |
|
I see why it doesn't need a PR approval because this is not going to merge into |
Done |
I based this PR on the PR for batch increasing sessions in the pool, as that PR already includes the dependencies and setup for the benchmarking. It's best to merge that PR first into master, and then rebase this PR on master once that's been done. |
olavloite
force-pushed
the
batch-increase-sessions
branch
from
edfa509
to
e3ac294
Compare
hengfengli
reviewed
Apr 6, 2020
There was a problem hiding this comment.
LGTM. @skuruppu please take a quick look because this PR is quite large.
olavloite
force-pushed
the
gc-sessions-instead-of-delete
branch
from
6401cb9
to
c95aed6
Compare
olavloite
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
skuruppu
approved these changes
Apr 15, 2020
Comment on lines
+1060
to
+1064
| sessionToKeepAlive = findSessionToKeepAlive(readSessions, keepAliveThreshold, 0); | ||
| if (sessionToKeepAlive == null) { | ||
| sessionToKeepAlive = findSessionToKeepAlive(writePreparedSessions, keepAliveThreshold); | ||
| sessionToKeepAlive = | ||
| findSessionToKeepAlive( | ||
| writePreparedSessions, keepAliveThreshold, readSessions.size()); |
There was a problem hiding this comment.
Doesn't this logic meant that over time we won't be maintaining the ratio of write prepared sessions to read sessions since we always favor keeping read sessions alive?
There was a problem hiding this comment.
Yes, the session that is kept alive is taken from the pool, pinged and then released back into the pool (on line 1074). Releasing the session back into the pool will trigger a new prepareSession call if necessary.
I've added an additional test case to verify this behavior.
There was a problem hiding this comment.
And that test case now shows that my line of thought does not always work as expected :-)
olavloite
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
When more sessions are requested by the user application than are available in the session pool, the session pool will now create new sessions in batches instead of in steps of 1. This reduces the number of RPCs needed to serve a burst of requests. A benchmark for the session pool has also been added to be able to compare performance and the number of RPCs needed before and after this change. This benchmark can also be used for future changes to verify that the change does not deteriorate performance or increase the number of RPCs needed.
olavloite
force-pushed
the
gc-sessions-instead-of-delete
branch
from
6ede807
to
a0dfa08
Compare
olavloite
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
olavloite
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
olavloite
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
olavloite
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
|
Thanks for the changes @olavloite. Please merge if you think it's ready to go. After that, I'll try to cut a release since we haven't done one in a while. |
gcf-merge-on-green bot
pushed a commit
that referenced
this pull request
Apr 22, 2020
🤖 I have created a release \*beep\* \*boop\* --- ## [1.53.0](https://www.github.com/googleapis/java-spanner/compare/v1.52.0...v1.53.0) (2020-04-22) ### Features * optimize maintainer to let sessions be GC'ed instead of deleted ([#135](https://www.github.com/googleapis/java-spanner/issues/135)) ([d65747c](https://www.github.com/googleapis/java-spanner/commit/d65747cbc704508f6f1bcef6eea53aa411d42ee2)) ### Bug Fixes * assign unique id's per test case ([#129](https://www.github.com/googleapis/java-spanner/issues/129)) ([a553b6d](https://www.github.com/googleapis/java-spanner/commit/a553b6d48c4f5ee2d0583e5b825d73a85f06216e)) * check for not null input for Id classes ([#159](https://www.github.com/googleapis/java-spanner/issues/159)) ([ecf5826](https://www.github.com/googleapis/java-spanner/commit/ecf582670818f32e85f534ec400d0b8d31cf9ca6)), closes [#145](https://www.github.com/googleapis/java-spanner/issues/145) * clean up test instance if creation failed ([#162](https://www.github.com/googleapis/java-spanner/issues/162)) ([ff571e1](https://www.github.com/googleapis/java-spanner/commit/ff571e16a45fbce692d9bb172749ff15fafe7a9c)) * fix flaky test and remove warnings ([#153](https://www.github.com/googleapis/java-spanner/issues/153)) ([d534e35](https://www.github.com/googleapis/java-spanner/commit/d534e350346b0c9ab8057ede36bc3aac473c0b06)), closes [#146](https://www.github.com/googleapis/java-spanner/issues/146) * increase test timeout and remove warnings ([#160](https://www.github.com/googleapis/java-spanner/issues/160)) ([63a6bd8](https://www.github.com/googleapis/java-spanner/commit/63a6bd8be08a56d002f58bc2cdb2856ad0dc5fa3)), closes [#158](https://www.github.com/googleapis/java-spanner/issues/158) * retry non-idempotent long-running RPCs ([#141](https://www.github.com/googleapis/java-spanner/issues/141)) ([4669c02](https://www.github.com/googleapis/java-spanner/commit/4669c02a24e0f7b1d53c9edf5ab7b146b4116960)) * retry restore if blocked by pending restore ([#119](https://www.github.com/googleapis/java-spanner/issues/119)) ([220653d](https://www.github.com/googleapis/java-spanner/commit/220653d8e25c518d0df447bf777a7fcbf04a01ca)), closes [#118](https://www.github.com/googleapis/java-spanner/issues/118) * StatementParser did not accept multiple query hints ([#170](https://www.github.com/googleapis/java-spanner/issues/170)) ([ef41a6e](https://www.github.com/googleapis/java-spanner/commit/ef41a6e503f218c00c16914aa9c1433d9b26db13)), closes [#163](https://www.github.com/googleapis/java-spanner/issues/163) * wait for initialization to finish before test ([#161](https://www.github.com/googleapis/java-spanner/issues/161)) ([fe434ff](https://www.github.com/googleapis/java-spanner/commit/fe434ff7068b4b618e70379c224e1c5ab88f6ba1)), closes [#146](https://www.github.com/googleapis/java-spanner/issues/146) ### Performance Improvements * increase sessions in the pool in batches ([#134](https://www.github.com/googleapis/java-spanner/issues/134)) ([9e5a1cd](https://www.github.com/googleapis/java-spanner/commit/9e5a1cdaacf71147b67681861f063c3276705f44)) * prepare sessions with r/w tx in-process ([#152](https://www.github.com/googleapis/java-spanner/issues/152)) ([2db27ce](https://www.github.com/googleapis/java-spanner/commit/2db27ce048efafaa3c28b097de33518747011465)), closes [#151](https://www.github.com/googleapis/java-spanner/issues/151) ### Dependencies * update core dependencies ([#109](https://www.github.com/googleapis/java-spanner/issues/109)) ([5753f1f](https://www.github.com/googleapis/java-spanner/commit/5753f1f4fed83df87262404f7a7ba7eedcd366cb)) * update core dependencies ([#132](https://www.github.com/googleapis/java-spanner/issues/132)) ([77c1558](https://www.github.com/googleapis/java-spanner/commit/77c1558652ee00e529674ac3a2dcf3210ef049fa)) * update dependency com.google.api:api-common to v1.9.0 ([#127](https://www.github.com/googleapis/java-spanner/issues/127)) ([b2c744f](https://www.github.com/googleapis/java-spanner/commit/b2c744f01a4d5a8981df5ff900f3536c83265a61)) * update dependency com.google.guava:guava-bom to v29 ([#147](https://www.github.com/googleapis/java-spanner/issues/147)) ([3fe3ae0](https://www.github.com/googleapis/java-spanner/commit/3fe3ae02376af552564c93c766f562d6454b7ac1)) * update dependency io.grpc:grpc-bom to v1.29.0 ([#164](https://www.github.com/googleapis/java-spanner/issues/164)) ([2d2ce5c](https://www.github.com/googleapis/java-spanner/commit/2d2ce5ce4dc8f410ec671e542e144d47f39ab40b)) * update dependency org.threeten:threetenbp to v1.4.3 ([#120](https://www.github.com/googleapis/java-spanner/issues/120)) ([49d1abc](https://www.github.com/googleapis/java-spanner/commit/49d1abcb6c9c48762dcf0fe1466ab107bf67146b)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).
rajatbhatta
pushed a commit
to rajatbhatta/java-spanner
that referenced
this pull request
Nov 17, 2022
…onfig to v0.6.0 (googleapis#135) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [com.google.cloud:google-cloud-shared-config](https://togithub.com/googleapis/java-shared-config) | minor | `0.5.0` -> `0.6.0` | --- ### Release Notes <details> <summary>googleapis/java-shared-config</summary> ### [`v0.6.0`](https://togithub.com/googleapis/java-shared-config/blob/master/CHANGELOG.md#​060-httpswwwgithubcomgoogleapisjava-shared-configcomparev050v060-2020-05-19) [Compare Source](https://togithub.com/googleapis/java-shared-config/compare/v0.5.0...v0.6.0) ##### Features - add new configuration necessary to support auto-value ([#​136](https://www.github.com/googleapis/java-shared-config/issues/136)) ([c14689b](https://www.github.com/googleapis/java-shared-config/commit/c14689b8791c173687f719d73156a989aedd7ba6)) </details> --- ### Renovate configuration :date: **Schedule**: At any time (no schedule defined). :vertical_traffic_light: **Automerge**: Disabled by config. Please merge this manually once you are satisfied. :recycle: **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. :no_bell: **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#googleapis/java-spanner-jdbc).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Optimizes the session pool maintainer to:
MinSessions + MaxIdleSessionsalive in the session pool. Let all other sessions automatically expire and remove these from the pool if they are not being used. This reduces the required number of RPCs to keep sessions alive.