Conversation

olavloite

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.burstWrite14683.78
SessionPoolBenchmark.burstWrite104800.407
SessionPoolBenchmark.burstWrite204763.488
SessionPoolBenchmark.burstWrite254741.395
SessionPoolBenchmark.burstWrite304687.498
SessionPoolBenchmark.burstWrite404740.128
SessionPoolBenchmark.burstWrite504723.155
SessionPoolBenchmark.burstWrite1004779.078

Benchmarks - After change

Benchmark(incStep)Time (ms)
SessionPoolBenchmark.burstWrite11191.581
SessionPoolBenchmark.burstWrite10994.402
SessionPoolBenchmark.burstWrite20955.75
SessionPoolBenchmark.burstWrite25916.664
SessionPoolBenchmark.burstWrite30898.538
SessionPoolBenchmark.burstWrite40952.803
SessionPoolBenchmark.burstWrite50906.222
SessionPoolBenchmark.burstWrite1001151.83

@googlebotgooglebot added the cla: yesThis human has signed the Contributor License Agreement.label Apr 14, 2020
@skuruppuskuruppu self-requested a review April 15, 2020 10:23
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
@olavloiteolavloite force-pushed the prepare-session-in-process branch from 8cb79a5 to 9e08ee3 Compare April 16, 2020 12:24
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
Collaborator Author

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.

@olavloiteolavloite merged commit 2db27ce into master Apr 22, 2020
@olavloiteolavloite deleted the prepare-session-in-process branch April 22, 2020 06:11
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..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..com/googleapis/java-spanner/issues/135)) ([d65747c](https://www..com/googleapis/java-spanner/commit/d65747cbc704508f6f1bcef6eea53aa411d42ee2))


### Bug Fixes

* assign unique id's per test case ([#129](https://www..com/googleapis/java-spanner/issues/129)) ([a553b6d](https://www..com/googleapis/java-spanner/commit/a553b6d48c4f5ee2d0583e5b825d73a85f06216e))
* check for not null input for Id classes ([#159](https://www..com/googleapis/java-spanner/issues/159)) ([ecf5826](https://www..com/googleapis/java-spanner/commit/ecf582670818f32e85f534ec400d0b8d31cf9ca6)), closes [#145](https://www..com/googleapis/java-spanner/issues/145)
* clean up test instance if creation failed ([#162](https://www..com/googleapis/java-spanner/issues/162)) ([ff571e1](https://www..com/googleapis/java-spanner/commit/ff571e16a45fbce692d9bb172749ff15fafe7a9c))
* fix flaky test and remove warnings ([#153](https://www..com/googleapis/java-spanner/issues/153)) ([d534e35](https://www..com/googleapis/java-spanner/commit/d534e350346b0c9ab8057ede36bc3aac473c0b06)), closes [#146](https://www..com/googleapis/java-spanner/issues/146)
* increase test timeout and remove warnings ([#160](https://www..com/googleapis/java-spanner/issues/160)) ([63a6bd8](https://www..com/googleapis/java-spanner/commit/63a6bd8be08a56d002f58bc2cdb2856ad0dc5fa3)), closes [#158](https://www..com/googleapis/java-spanner/issues/158)
* retry non-idempotent long-running RPCs ([#141](https://www..com/googleapis/java-spanner/issues/141)) ([4669c02](https://www..com/googleapis/java-spanner/commit/4669c02a24e0f7b1d53c9edf5ab7b146b4116960))
* retry restore if blocked by pending restore ([#119](https://www..com/googleapis/java-spanner/issues/119)) ([220653d](https://www..com/googleapis/java-spanner/commit/220653d8e25c518d0df447bf777a7fcbf04a01ca)), closes [#118](https://www..com/googleapis/java-spanner/issues/118)
* StatementParser did not accept multiple query hints ([#170](https://www..com/googleapis/java-spanner/issues/170)) ([ef41a6e](https://www..com/googleapis/java-spanner/commit/ef41a6e503f218c00c16914aa9c1433d9b26db13)), closes [#163](https://www..com/googleapis/java-spanner/issues/163)
* wait for initialization to finish before test ([#161](https://www..com/googleapis/java-spanner/issues/161)) ([fe434ff](https://www..com/googleapis/java-spanner/commit/fe434ff7068b4b618e70379c224e1c5ab88f6ba1)), closes [#146](https://www..com/googleapis/java-spanner/issues/146)


### Performance Improvements

* increase sessions in the pool in batches ([#134](https://www..com/googleapis/java-spanner/issues/134)) ([9e5a1cd](https://www..com/googleapis/java-spanner/commit/9e5a1cdaacf71147b67681861f063c3276705f44))
* prepare sessions with r/w tx in-process ([#152](https://www..com/googleapis/java-spanner/issues/152)) ([2db27ce](https://www..com/googleapis/java-spanner/commit/2db27ce048efafaa3c28b097de33518747011465)), closes [#151](https://www..com/googleapis/java-spanner/issues/151)


### Dependencies

* update core dependencies ([#109](https://www..com/googleapis/java-spanner/issues/109)) ([5753f1f](https://www..com/googleapis/java-spanner/commit/5753f1f4fed83df87262404f7a7ba7eedcd366cb))
* update core dependencies ([#132](https://www..com/googleapis/java-spanner/issues/132)) ([77c1558](https://www..com/googleapis/java-spanner/commit/77c1558652ee00e529674ac3a2dcf3210ef049fa))
* update dependency com.google.api:api-common to v1.9.0 ([#127](https://www..com/googleapis/java-spanner/issues/127)) ([b2c744f](https://www..com/googleapis/java-spanner/commit/b2c744f01a4d5a8981df5ff900f3536c83265a61))
* update dependency com.google.guava:guava-bom to v29 ([#147](https://www..com/googleapis/java-spanner/issues/147)) ([3fe3ae0](https://www..com/googleapis/java-spanner/commit/3fe3ae02376af552564c93c766f562d6454b7ac1))
* update dependency io.grpc:grpc-bom to v1.29.0 ([#164](https://www..com/googleapis/java-spanner/issues/164)) ([2d2ce5c](https://www..com/googleapis/java-spanner/commit/2d2ce5ce4dc8f410ec671e542e144d47f39ab40b))
* update dependency org.threeten:threetenbp to v1.4.3 ([#120](https://www..com/googleapis/java-spanner/issues/120)) ([49d1abc](https://www..com/googleapis/java-spanner/commit/49d1abcb6c9c48762dcf0fe1466ab107bf67146b))
---


This PR was generated with [Release Please](https://.com/googleapis/release-please).
rajatbhatta pushed a commit to rajatbhatta/java-spanner that referenced this pull request Nov 17, 2022
)

Add documentation for properties that are supported for end users.
This change does not add documentation for the userAgent property, as
it is not a property that should be set by end users. This property
should only be set by libraries that are maintained by Google.

Fixes googleapis#152
Sign up for free to join this conversation on . Already have an account? Sign in to comment
cla: yesThis human has signed the Contributor License Agreement.
None yet

Successfully merging this pull request may close these issues.

Prepare sessions for r/w tx can be bottleneck