Skip to content

Conversation

olavloite
Copy link
Collaborator

Adds a number of safeguards against hanging transactions if the first statement of a transaction for whatever reason fails to return a transaction ID, or otherwise returns an unexpected error. This PR contains the following safeguards:

  1. If a statement requested a new transaction from Cloud Spanner and receives a response without a transaction ID, the statement will throw an exception. This should theoretically never happen. If it did happen, it would cause the second statement in the transaction to be 'stuck', as it would wait indefinitely for the first statement to return a transaction.
  2. If the first statement of a transaction throws an unexpected error (a Throwable that is not a SpannerException), the exception will be translated to a SpannerException and handled as such. This error will mark the transaction as unusable, as the first statement failed to return a transaction.
  3. If the first statement fails to respond at all (no response, no error), the second statement will wait at most 60 seconds for a transaction ID to be returned, and then fail with a ABORTED error. This will cause the transaction to be retried automatically.

Fixes #799

@olavloite olavloite requested a review from a team as a code owner January 14, 2021 15:48
@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/java-spanner API. label Jan 14, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jan 14, 2021
@olavloite olavloite added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 14, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 14, 2021
@codecov
Copy link

codecov bot commented Jan 14, 2021

Codecov Report

Merging #800 (02a886b) into master (1a71e50) will increase coverage by 0.08%.
The diff coverage is 92.85%.

Impacted file tree graph

@@ Coverage Diff @@ ## master #800 +/- ## ============================================ + Coverage 85.01% 85.09% +0.08%  - Complexity 2562 2564 +2  ============================================ Files 143 143 Lines 14015 14030 +15 Branches 1341 1341 ============================================ + Hits 11915 11939 +24  + Misses 1537 1531 -6  + Partials 563 560 -3 
Impacted Files Coverage Δ Complexity Δ
...om/google/cloud/spanner/TransactionRunnerImpl.java 85.74% <92.10%> (+0.87%) 9.00 <0.00> (ø)
.../com/google/cloud/spanner/AbstractReadContext.java 87.07% <100.00%> (ø) 48.00 <1.00> (ø)
...va/com/google/cloud/spanner/AbstractResultSet.java 83.30% <100.00%> (+0.03%) 28.00 <0.00> (ø)
...ain/java/com/google/cloud/spanner/SessionPool.java 89.02% <0.00%> (+0.19%) 71.00% <0.00%> (ø%)
...ud/spanner/SessionPoolAsyncTransactionManager.java 87.30% <0.00%> (+1.58%) 13.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 1a71e50...02a886b. Read the comment docs.

@olavloite olavloite changed the title fix: safeguard against statements errors when requesting tx fix: safeguard against statement errors when requesting a transaction Jan 14, 2021
Copy link
Contributor

@thiagotnunes thiagotnunes left a comment

Choose a reason for hiding this comment

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

Thanks for this fixes @olavloite, really appreciate it.

@thiagotnunes thiagotnunes merged commit c4776e4 into master Jan 14, 2021
@thiagotnunes thiagotnunes deleted the issue-799 branch January 14, 2021 23:01
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#5930 Changes: docs(servicemanagement): fix remaining broken links PiperOrigin-RevId: 443508623 Source-Link: googleapis/googleapis@fd6935f feat(channel): Add new enum value, new filter in ListCustomersRequest of Cloud Channel API PiperOrigin-RevId: 443474187 Source-Link: googleapis/googleapis@d4dd268
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.

3 participants