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

fix(3.1.x): UNAVAILABLE error on first query could cause transaction to get stuck #856

Merged

Conversation

thiagotnunes
Copy link
Contributor

If the first query or read operation of a read/write transaction would return UNAVAILABLE for the first element of the result stream, the transaction could get stuck. This was caused by the internal retry mechanism that would wait for the initial attempt to return a transaction, which was never returned as the UNAVAILABLE exception was internally handled by the result stream iterator.

Fixes #799

…stuck (googleapis#807)

If the first query or read operation of a read/write transaction would return UNAVAILABLE for
the first element of the result stream, the transaction could get stuck. This was caused by the
internal retry mechanism that would wait for the initial attempt to return a transaction, which
was never returned as the UNAVAILABLE exception was internally handled by the result stream
iterator.

Fixes googleapis#799
@thiagotnunes thiagotnunes requested a review from a team as a code owner February 8, 2021 02:45
@google-cla
Copy link

google-cla bot commented Feb 8, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/java-spanner API. label Feb 8, 2021
@google-cla google-cla bot added the cla: no This human has *not* signed the Contributor License Agreement. label Feb 8, 2021
@codecov
Copy link

codecov bot commented Feb 8, 2021

Codecov Report

Merging #856 (4c786f1) into 3.1.x (0fd859d) will increase coverage by 0.00%.
The diff coverage is 88.88%.

Impacted file tree graph

@@            Coverage Diff            @@
##              3.1.x     #856   +/-   ##
=========================================
  Coverage     85.05%   85.05%           
+ Complexity     2556     2554    -2     
=========================================
  Files           142      142           
  Lines         13930    13940   +10     
  Branches       1326     1330    +4     
=========================================
+ Hits          11848    11857    +9     
+ Misses         1526     1525    -1     
- Partials        556      558    +2     
Impacted Files Coverage Δ Complexity Δ
...va/com/google/cloud/spanner/AbstractResultSet.java 83.48% <ø> (ø) 28.00 <0.00> (ø)
.../com/google/cloud/spanner/AbstractReadContext.java 86.56% <87.50%> (-0.21%) 47.00 <0.00> (ø)
...om/google/cloud/spanner/TransactionRunnerImpl.java 84.27% <100.00%> (ø) 9.00 <0.00> (ø)
...ud/spanner/SessionPoolAsyncTransactionManager.java 85.36% <0.00%> (-1.63%) 11.00% <0.00%> (-2.00%)
...m/google/cloud/spanner/connection/SpannerPool.java 86.11% <0.00%> (+1.66%) 31.00% <0.00%> (ø%)

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 0fd859d...4fe2ddf. Read the comment docs.

@google-cla
Copy link

google-cla bot commented Feb 8, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@thiagotnunes thiagotnunes changed the title fix: UNAVAILABLE error on first query could cause transaction to get stuck fix(3.1.x): UNAVAILABLE error on first query could cause transaction to get stuck Feb 8, 2021
Comment on lines 538 to 539
* <li>The default {@link SpannerOptions#getDefaultQueryOptions()} specified for the database
* where the query is executed.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This does not seem to be part of this change.

@@ -1071,6 +1071,7 @@ protected PartialResultSet computeNext() {
backoffSleep(context, backOff);
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: Not part of this change

@google-cla
Copy link

google-cla bot commented Feb 9, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@thiagotnunes thiagotnunes added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Feb 9, 2021
@thiagotnunes thiagotnunes merged commit a346eb3 into googleapis:3.1.x Feb 9, 2021
@thiagotnunes thiagotnunes deleted the unavailable-first-query-3.1.x branch February 9, 2021 02:30
ansh0l pushed a commit to ansh0l/java-spanner that referenced this pull request Nov 10, 2022
This is an auto-generated regeneration of the .pb.go files by
cloud.google.com/go/internal/gapicgen. Once this PR is submitted, genbot will
update the corresponding PR to depend on the newer version of go-genproto, and
assign reviewers. Whilst this or any regen PR is open in go-genproto, genbot
will not create any more regeneration PRs. If all regen PRs are closed,
gapicgen will create a new set of regeneration PRs once per night.

If you have been assigned to review this PR, please:

- Ensure that CI is passing. If it's failing, it requires your manual attention.
- Approve and submit this PR if you believe it's ready to ship. That will prompt
genbot to assign reviewers to the google-cloud-go PR.

Corresponding google-cloud-go PR: googleapis/google-cloud-go#6333

Changes:

chore(python): prepare for the v2beta release of iam python client
  PiperOrigin-RevId: 460590348
  Source-Link: googleapis/googleapis@5688bdb

feat(dlp): InfoType categories were added to built-in infoTypes
  PiperOrigin-RevId: 460542545
  Source-Link: googleapis/googleapis@898fcea

feat(dataform): add fields for tracking reachable status of resources in list methods feat: add the failure_reason field to workflow invocation actions
  PiperOrigin-RevId: 460402867
  Source-Link: googleapis/googleapis@5c90074

feat(networkmanagement): add new abort cause and new route next hop type
  PiperOrigin-RevId: 460402186
  Source-Link: googleapis/googleapis@5813dcf

chore(bazel): move rules_gapic up in WORKSPACE loading order
  PiperOrigin-RevId: 460327690
  Source-Link: googleapis/googleapis@f86f3a2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/java-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.

None yet

2 participants