feat: optimize maintainer to let sessions be GC'ed instead of deleted #135

olavloite · 2020-03-30T09:22:18Z

Optimizes the session pool maintainer to:

Let the backend garbage collect sessions instead of explicitly deleting them. This reduces the number of RPCs needed.
Only keep MinSessions + MaxIdleSessions alive 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.

hengfengli · 2020-04-01T23:54:35Z

@skuruppu This repo does not need a PR approval before merging. Do we need to change the config?

hengfengli · 2020-04-02T00:03:56Z

@olavloite (3) can be reverted because it turns out SELECT 1 is better.

hengfengli · 2020-04-02T00:05:58Z

I see why it doesn't need a PR approval because this is not going to merge into master. Only merging into master requires an approval.

olavloite · 2020-04-02T09:06:25Z

@olavloite (3) can be reverted because it turns out SELECT 1 is better.

Done

olavloite · 2020-04-02T09:07:39Z

I see why it doesn't need a PR approval because this is not going to merge into master. Only merging into master requires an approval.

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.

hengfengli

LGTM. @skuruppu please take a quick look because this PR is quite large.

skuruppu · 2020-04-15T09:45:32Z

google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java

+          sessionToKeepAlive = findSessionToKeepAlive(readSessions, keepAliveThreshold, 0);
          if (sessionToKeepAlive == null) {
-            sessionToKeepAlive = findSessionToKeepAlive(writePreparedSessions, keepAliveThreshold);
+            sessionToKeepAlive =
+                findSessionToKeepAlive(
+                    writePreparedSessions, keepAliveThreshold, readSessions.size());


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?

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.

And that test case now shows that my line of thought does not always work as expected :-)

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.

skuruppu · 2020-04-21T23:19:05Z

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.

…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. --- - [ ] 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).

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 30, 2020

olavloite added the api: spanner Issues related to the googleapis/java-spanner API. label Mar 30, 2020

olavloite requested review from skuruppu and hengfengli March 30, 2020 09:56

olavloite force-pushed the batch-increase-sessions branch from edfa509 to e3ac294 Compare April 4, 2020 15:10

hengfengli reviewed Apr 6, 2020

View reviewed changes

olavloite changed the base branch from batch-increase-sessions to master April 8, 2020 05:00

olavloite force-pushed the gc-sessions-instead-of-delete branch from 6401cb9 to c95aed6 Compare April 8, 2020 05:28

olavloite added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 8, 2020

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 8, 2020

skuruppu approved these changes Apr 15, 2020

View reviewed changes

olavloite added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 16, 2020

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 16, 2020

olavloite added 9 commits April 21, 2020 12:24

fix: remove unused code

424d370

fix: include num rpcs and sessions in benchmark results

fa222f2

fix: include num rpcs and sessions in benchmark results

4d96c2f

fix: revert using GetSession as ping

87af068

fix: merge conflicts

b5c88dc

tests: add test to verify writeFraction

c7d307e

fix: sessions being prepared should be included in comparison

9df4713

fix: update renamed mock server

a0dfa08

olavloite force-pushed the gc-sessions-instead-of-delete branch from 6ede807 to a0dfa08 Compare April 21, 2020 10:31

olavloite added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 21, 2020

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 21, 2020

olavloite added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 21, 2020

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 21, 2020

olavloite added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 21, 2020

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 21, 2020

olavloite added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 21, 2020

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 21, 2020

skuruppu merged commit d65747c into master Apr 22, 2020

skuruppu deleted the gc-sessions-instead-of-delete branch April 22, 2020 01:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: optimize maintainer to let sessions be GC'ed instead of deleted #135

feat: optimize maintainer to let sessions be GC'ed instead of deleted #135

olavloite commented Mar 30, 2020 •

edited

Loading

hengfengli commented Apr 1, 2020

hengfengli commented Apr 2, 2020 •

edited

Loading

hengfengli commented Apr 2, 2020

olavloite commented Apr 2, 2020

olavloite commented Apr 2, 2020

hengfengli left a comment

skuruppu Apr 15, 2020

olavloite Apr 16, 2020

olavloite Apr 16, 2020

skuruppu commented Apr 21, 2020

feat: optimize maintainer to let sessions be GC'ed instead of deleted #135

feat: optimize maintainer to let sessions be GC'ed instead of deleted #135

Conversation

olavloite commented Mar 30, 2020 • edited Loading

hengfengli commented Apr 1, 2020

hengfengli commented Apr 2, 2020 • edited Loading

hengfengli commented Apr 2, 2020

olavloite commented Apr 2, 2020

olavloite commented Apr 2, 2020

hengfengli left a comment

Choose a reason for hiding this comment

skuruppu Apr 15, 2020

Choose a reason for hiding this comment

olavloite Apr 16, 2020

Choose a reason for hiding this comment

olavloite Apr 16, 2020

Choose a reason for hiding this comment

skuruppu commented Apr 21, 2020

olavloite commented Mar 30, 2020 •

edited

Loading

hengfengli commented Apr 2, 2020 •

edited

Loading