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

perf: prepare sessions with r/w tx in-process #152

Merged
merged 3 commits into from Apr 22, 2020

Conversation

olavloite
Copy link
Contributor

@olavloite olavloite commented Apr 14, 2020

Preparing sessions with a read/write transaction using a background executor works well as long as that executor is not being overloaded. When the executor has reached its limit, it is more efficient to allow the read/write transaction to be created in-process, as that scales with the number of user threads available, instead of being limited to the fixed thread pool of the background executor.

Fixes #151

Benchmarks - Before change

Benchmark (incStep) Time (ms)
SessionPoolBenchmark.burstWrite 1 4683.78
SessionPoolBenchmark.burstWrite 10 4800.407
SessionPoolBenchmark.burstWrite 20 4763.488
SessionPoolBenchmark.burstWrite 25 4741.395
SessionPoolBenchmark.burstWrite 30 4687.498
SessionPoolBenchmark.burstWrite 40 4740.128
SessionPoolBenchmark.burstWrite 50 4723.155
SessionPoolBenchmark.burstWrite 100 4779.078

Benchmarks - After change

Benchmark (incStep) Time (ms)
SessionPoolBenchmark.burstWrite 1 1191.581
SessionPoolBenchmark.burstWrite 10 994.402
SessionPoolBenchmark.burstWrite 20 955.75
SessionPoolBenchmark.burstWrite 25 916.664
SessionPoolBenchmark.burstWrite 30 898.538
SessionPoolBenchmark.burstWrite 40 952.803
SessionPoolBenchmark.burstWrite 50 906.222
SessionPoolBenchmark.burstWrite 100 1151.83

@googlebot googlebot added the cla: yes label Apr 14, 2020
@skuruppu skuruppu self-requested a review Apr 15, 2020
Preparing sessions with a read/write transaction using a background
executor works well as long as that executor is not being overloaded.
When the executor has reached its limit, it is more efficient to allow
the read/write transaction to be created in-process, as that scales
with the number of user threads available, instead of being limited to
the fixed thread pool of the background executor.

Fixes #151
@olavloite olavloite force-pushed the prepare-session-in-process branch from 8cb79a5 to 9e08ee3 Compare Apr 16, 2020
readWaiters.add(waiter);
} else {
readWriteWaiters.add(waiter);
}
Copy link
Contributor

@skuruppu skuruppu Apr 22, 2020

Choose a reason for hiding this comment

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

I don't quite understand this. Why is the waiter being added to readWaiters if inProcessPrepare? Aren't we trying to create a read/write session here?

I suppose I don't understand why it makes a difference if this is done in-process.

Copy link
Contributor Author

@olavloite olavloite Apr 22, 2020

Choose a reason for hiding this comment

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

I've added a comment on why we add it to readWaiters here.

The reasons are:

  1. The normal prepare process is executed using a background executor with a fixed thread pool. When the work queue for this executor has exceeded the number of threads available in the thread pool, any requester for a read/write session will have to wait for a thread to come available, and then for the prepare to actually be executed. In the meantime, the user thread is just waiting. In those cases, it's more efficient to use the user thread to execute the BeginTransaction RPC, instead of letting it wait for a thread to come available in the thread pool.
  2. In this specific case, in addition to the work queue for preparing sessions being longer than the number of threads in the thread pool, there is also no session at all available in the session pool. In that case, it's more efficient to wait for any (read) session to come available, and then execute the BeginTransaction RPC using the user thread, than to wait for both a session to come available, and then for a thread in the thread pool to come available for preparing the session.

@olavloite olavloite merged commit 2db27ce into master Apr 22, 2020
12 of 13 checks passed
@olavloite olavloite deleted the prepare-session-in-process branch Apr 22, 2020
gcf-merge-on-green bot pushed a commit that referenced this issue 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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants