Conversation

olavloite

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

@olavloiteolavloite requested a review from a team as a code owner January 14, 2021 15:48
@product-auto-labelproduct-auto-label bot added the api: spannerIssues related to the googleapis/java-spanner API.label Jan 14, 2021
@google-clagoogle-cla bot added the cla: yesThis human has signed the Contributor License Agreement.label Jan 14, 2021
@olavloiteolavloite added the kokoro:force-runAdd this label to force Kokoro to re-run the tests.label Jan 14, 2021
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-runAdd this label to force Kokoro to re-run the tests.label Jan 14, 2021
@codecov

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 FilesCoverage ΔComplexity Δ
...om/google/cloud/spanner/TransactionRunnerImpl.java85.74% <92.10%> (+0.87%)9.00 <0.00> (ø)
.../com/google/cloud/spanner/AbstractReadContext.java87.07% <100.00%> (ø)48.00 <1.00> (ø)
...va/com/google/cloud/spanner/AbstractResultSet.java83.30% <100.00%> (+0.03%)28.00 <0.00> (ø)
...ain/java/com/google/cloud/spanner/SessionPool.java89.02% <0.00%> (+0.19%)71.00% <0.00%> (ø%)
...ud/spanner/SessionPoolAsyncTransactionManager.java87.30% <0.00%> (+1.58%)13.00% <0.00%> (+2.00%)
...m/google/cloud/spanner/connection/SpannerPool.java86.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.

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

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.

@thiagotnunesthiagotnunes merged commit c4776e4 into master Jan 14, 2021
@thiagotnunesthiagotnunes 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 . Already have an account? Sign in to comment
api: spannerIssues related to the googleapis/java-spanner API.cla: yesThis human has signed the Contributor License Agreement.
None yet

Successfully merging this pull request may close these issues.

Spanner: TransactionContext hangs thread indefinitely