Skip to content

Create Sessions Async #1734

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.

Sign up for GitHub

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

Jump to bottom
Merged
dwsupplee merged 15 commits into from
Apr 15, 2019
Merged

Create Sessions Async #1734

dwsupplee merged 15 commits into from
Apr 15, 2019

Conversation

@danielgsims
Copy link
Contributor

@danielgsims danielgsims commented Mar 18, 2019 •
edited by jdpedrie
Loading

  • Adds additional methods to Database, Operation and Transport to create sessions asynchronously
  • CacheSessionPool warm up will create sessions asynchronously

Closes #1004.

@danielgsims danielgsims requested a review from jdpedrie as a code owner March 18, 2019 01:16
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 18, 2019
@codecov
Copy link

codecov bot commented Mar 18, 2019 •
edited
Loading

Codecov Report

Merging #1734 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1734      +/-   ##
============================================
+ Coverage      91.5%   91.52%   +0.02%     
- Complexity     4352     4365      +13     
============================================
  Files           303      304       +1     
  Lines         12849    12892      +43     
============================================
+ Hits          11757    11800      +43     
  Misses         1092     1092
Impacted Files Coverage Δ Complexity Δ
Spanner/src/Session/CacheSessionPool.php 97.56% <100%> (+0.07%) 89 <0> (+3) ⬆️
Spanner/src/Connection/Grpc.php 93.77% <100%> (+0.26%) 89 <2> (+2) ⬆️
Firestore/src/FieldValue/ArrayRemoveValue.php 100% <0%> (ø) 3% <0%> (+1%) ⬆️
Firestore/src/FieldValue.php 100% <0%> (ø) 5% <0%> (+1%) ⬆️
Firestore/src/FieldValue/ArrayUnionValue.php 100% <0%> (ø) 3% <0%> (+1%) ⬆️
Firestore/src/WriteBatch.php 100% <0%> (ø) 128% <0%> (+1%) ⬆️
Firestore/src/FieldValue/DeleteFieldValue.php 100% <0%> (ø) 1% <0%> (ø) ⬇️
Firestore/src/FieldValue/ServerTimestampValue.php 100% <0%> (ø) 4% <0%> (+1%) ⬆️
Firestore/src/FieldValue/IncrementValue.php 100% <0%> (ø) 3% <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 780b463...9bbef99. Read the comment docs.

dwsupplee
dwsupplee suggested changes Mar 18, 2019
View reviewed changes
Copy link
Contributor

@dwsupplee dwsupplee left a comment

Choose a reason for hiding this comment

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

Thanks @danielgsims, here's a cursory review with a few high level notes.

@danielgsims
Refactors creationg sessions asynchronously
52746a1 
  * Moves most of the logic into the Cache Session Pool
  * Removes any logic from Database and Operation
  * Grpc connection creates single session asynchronously
  * Updates tests
@danielgsims
Resets style formatting
319ab22
@danielgsims
Copy link
Contributor Author

danielgsims commented Mar 19, 2019

@dwsupplee - I made the changes. GrpcConnection now makes one request/promise per call via createSessionAsync. The CacheSessionPool is responsible for aggregating the promises and handling their responses. Let me know if you see anything else!

@danielgsims danielgsims requested a review from dwsupplee March 19, 2019 03:05
jdpedrie
jdpedrie suggested changes Mar 19, 2019
View reviewed changes
@codecov-io
Copy link

codecov-io commented Mar 20, 2019 •
edited by codecov bot
Loading

Codecov Report

Merging #1734 into master will increase coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1734      +/-   ##
============================================
+ Coverage      91.5%   91.54%   +0.04%     
- Complexity     4352     4372      +20     
============================================
  Files           303      304       +1     
  Lines         12849    12932      +83     
============================================
+ Hits          11757    11839      +82     
- Misses         1092     1093       +1
Impacted Files Coverage Δ Complexity Δ
Spanner/src/Session/CacheSessionPool.php 97.49% <100%> (ø) 88 <0> (+2) ⬆️
Spanner/src/Connection/Grpc.php 93.76% <100%> (+0.25%) 89 <2> (+2) ⬆️
Core/src/RequestWrapper.php 92.06% <0%> (-0.62%) 53% <0%> (+3%)
Vision/src/Annotation/AbstractFeature.php 100% <0%> (ø) 1% <0%> (ø) ⬇️
Vision/src/Annotation.php 100% <0%> (ø) 31% <0%> (ø) ⬇️
Vision/src/Image.php 100% <0%> (ø) 17% <0%> (ø) ⬇️
Core/src/ExponentialBackoff.php 100% <0%> (ø) 12% <0%> (+2%) ⬆️
Vision/src/Annotation/Web/WebImage.php 100% <0%> (ø) 1% <0%> (ø) ⬇️
Vision/src/Connection/Rest.php 100% <0%> (ø) 2% <0%> (ø) ⬇️
Vision/src/Annotation/Web/WebPage.php 100% <0%> (ø) 1% <0%> (ø) ⬇️
... and 25 more

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 780b463...ca128d0. Read the comment docs.

@danielgsims danielgsims requested a review from jdpedrie March 21, 2019 02:08
@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Mar 25, 2019
dwsupplee
dwsupplee suggested changes Mar 26, 2019
View reviewed changes
dwsupplee
dwsupplee reviewed Apr 10, 2019
View reviewed changes
dwsupplee
dwsupplee reviewed Apr 10, 2019
View reviewed changes
dwsupplee
dwsupplee reviewed Apr 10, 2019
View reviewed changes
dwsupplee
dwsupplee reviewed Apr 11, 2019
View reviewed changes
dwsupplee
dwsupplee reviewed Apr 11, 2019
View reviewed changes
@dwsupplee dwsupplee merged commit afc9ee3 into googleapis:master Apr 15, 2019
@jdpedrie jdpedrie mentioned this pull request Apr 16, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@jdpedrie jdpedrie jdpedrie approved these changes

@dwsupplee dwsupplee dwsupplee approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes This human has signed the Contributor License Agreement. 🚨 This issue needs some love.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Spanner] Making the session warmup parallel

6 participants

@danielgsims @codecov-io @jdpedrie @dwsupplee @googlebot @yoshi-automation