Skip to content

Spanner: Session pool should return sessions in LIFO order - #6178

Merged
olavloite merged 2 commits into
googleapis:masterfrom
olavloite:spanner-session-pool-lifo
Aug 28, 2019
Merged

Spanner: Session pool should return sessions in LIFO order#6178
olavloite merged 2 commits into
googleapis:masterfrom
olavloite:spanner-session-pool-lifo

Conversation

@olavloite

Copy link
Copy Markdown
Contributor

Makes the session pool return sessions in LIFO order, as this decreases the chance that a client is handed a session that has had its server side caches and connections closed. By returning sessions in LIFO order, the P99 latency for queries, especially in typical 1QPS load scenarios, is reduced.

Updates #6169

Make the session pool return sessions in LIFO order as this
decreases the chance that a client is handed a session that
has had its server side caches and connections closed. By
returning sessions in LIFO order, the P99 latency for queries,
especially in typical 1QPS load scenarios, is reduced.
Updates googleapis#6169
@googlebotgooglebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 27, 2019
session4.close();

Session rw1 = pool.getReadWriteSession();
Session rw2 = pool.getReadWriteSession();

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.

[nit] Naming wise, I would call these session5 and session6 to be consistent with the variable naming elsewhere in this test and file, and since the type is not relevant.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Renamed to session5 and session6.

@codecov

codecovBot commented Aug 28, 2019

Copy link
Copy Markdown

Codecov Report

Merging #6178 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@ Coverage Diff @@## master #6178 +/- ##
============================================
+ Coverage 47.5% 47.5% +<.01% - Complexity 27432 27447 +15 
============================================
Files 2523 2523 Lines 274592 274592 Branches 31401 31396 -5 ============================================
+ Hits 130439 130445 +6 + Misses 134520 134514 -6 
Partials 9633 9633
Impacted FilesCoverage ΔComplexity Δ
.../cloud/datastore/testing/LocalDatastoreHelper.java85.07% <0%> (+4.47%)17% <0%> (ø)⬇️
.../java/com/google/cloud/testing/CommandWrapper.java96.96% <0%> (+9.09%)13% <0%> (ø)⬇️

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 f48ee81...c64d0b8. Read the comment docs.

@olavloite
olavloite merged commit 172f7a8 into googleapis:masterAug 28, 2019
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yesThis human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@olavloite@skuruppu@googlebot