Skip to content
This repository was archived by the owner on Mar 4, 2026. It is now read-only.

perf: use write fraction when resizing pool#1031

Merged
olavloite merged 15 commits into
googleapis:masterfrom
olavloite:write-fraction
Jun 9, 2020
Merged

perf: use write fraction when resizing pool#1031
olavloite merged 15 commits into
googleapis:masterfrom
olavloite:write-fraction

Conversation

@olavloite

@olavloite olavloite commented May 24, 2020

Copy link
Copy Markdown
Contributor

Note: This PR depends on #963, which should be merged first.

The write fraction was only used when filling the pool, and not when creating individual sessions. Instead, the type of session that was created was determined based on the type that was needed at that time. That type would then also be remembered for the lifetime of the session. This is a good default when no write fraction has been set, but it is not logical if the user has configured a write
fraction.

This change ensures that the write fraction is respected when one is set. When no write fraction has been configured, the default behavior is retained, which means that the type of session is determined based on the type of session that is being created. It is however not 'sticky', which means that if the pool only contains write sessions, and a read session is needed, the pool will return a write session, but its type will be changed to read-only and it will not be prepared
for read/write transactions when it is returned to the pool. This improves latency for situations where a burst of write transactions has created a lot of write-prepared sessions in the pool that is followed by a sequence of read-only transactions.

This change reduces latency in 1-transaction-per-second scenarios where the user has not specified a value for min and writes greater than zero in the session pool configuration. The default value for both configuration options are zero.

The benchmarks for these scenarios are below. The TLDR version is: This change improves read/write transaction latency by approx 15ms, except when min and writes have both been configured to values above 0. In that case, there is no significant difference.

      Old Old New New
Scenario Min Writes Avg exec time P90 exec time Avg exec time P90 exec time
100 write transactions 0 0 73.16 81.17 54.97 60.00
100 write transactions 0 0.2 68.43 74.42 53.67 59.51
100 write transactions 25 0 66.37 72.96 53.28 58.12
100 write transactions 25 0.2 51.52 56.91 51.93 56.99
100 read-only transactions 0 0 34.90 39.26 32.43 37.61
100 read-only transactions 0 0.2 33.40 37.67 32.49 36.67
100 read-only transactions 25 0 29.32 34.03 32.69 38.62
100 read-only transactions 25 0.2 30.15 34.69 31.62 36.29
100 read + 100 write tx 0 0 51.77 73.65 44.58 58.96
100 read + 100 write tx 0 0.2 49.98 70.82 42.30 56.73
100 read + 100 write tx 25 0 49.75 70.97 42.14 56.88
100 read + 100 write tx 25 0.2 41.60 55.48 41.72 55.51

olavloite added 4 commits May 18, 2020 22:09
The write fraction was only used when filling the pool, and not when creating
individual sessions. Instead, the type of session that was created was determined
based on the type that was needed at that time. That type would then also be
remembered for the lifetime of the session. This is a good default when no write
fraction has been set, but it is not logical if the user has configured a write
fraction.
This change ensures that the write fraction is respected when one is set. When no
write fraction has been configured, the default behavior is retained, which means
that the type of session is determined based on the type of session that is being
created. It is however not 'sticky', which means that if the pool only contains
write sessions, and a read session is needed, the pool will return a write
session, but its type will be changed to read-only and it will not be prepared
for read/write transactions when it is returned to the pool. This improves
latency for situations where a burst of write transactions has created a lot of
write-prepared sessions in the pool that is followed by a sequence of read-only
transactions.
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 24, 2020
@olavloite olavloite added api: spanner Issues related to the googleapis/nodejs-spanner API. status: do not merge labels May 24, 2020
@olavloite olavloite changed the title Write fraction perf: use write fraction when resizing pool May 24, 2020
@codecov

codecov Bot commented May 24, 2020

Copy link
Copy Markdown

Codecov Report

Merging #1031 into master will increase coverage by 0.00%.
The diff coverage is 99.40%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #1031    +/-   ##
========================================
  Coverage   98.27%   98.27%            
========================================
  Files          21       21            
  Lines       20469    20602   +133     
  Branches     1094     1109    +15     
========================================
+ Hits        20115    20247   +132     
- Misses        351      352     +1     
  Partials        3        3            
Impacted Files Coverage Δ
src/session-pool.ts 99.91% <99.40%> (-0.09%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e4fbbb...7c3aac1. Read the comment docs.

@olavloite olavloite requested a review from skuruppu May 25, 2020 17:30
@skuruppu

Copy link
Copy Markdown
Contributor

Hope you don't mind if I review this after #963 is merged in.

Comment thread src/session-pool.ts
new Promise((_, reject) => {
this._createSessions({reads, writes}).catch(reject);
this._pending -= reads;
this._createSessions({reads, writes: 0}).catch(reject);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I got a little bit worried when I got here because I could no longer find where the write sessions are being created. Did I miss something?

Sorry if I misunderstood the change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I can understand the confusion. Instead of determining the number of write session to create here, it is determined when the session is released into the pool. That happens on line 568 in the release function. That way the pool can:

  1. Check whether there is anyone waiting for a session and give that session directly to that waiter, instead of first create a transaction for the session.
  2. Check the actual number of write sessions in the pool at that moment, and determine whether another write session is needed or not.
  3. Check whether a session was used as a write session during the last request and mark it as a session that should once again be prepared with a read/write transaction, even when the session pool configuration has the value writes: 0.

If shouldBeWrite is true and there is no process waiting for a session, the session will be marked as a write-session and a transaction will be prepared at the end of the release function (line 634).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great, thanks for the explanation @olavloite. That makes sense.

@olavloite olavloite merged commit 58f773b into googleapis:master Jun 9, 2020
@olavloite olavloite deleted the write-fraction branch June 9, 2020 15:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: spanner Issues related to the googleapis/nodejs-spanner API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants