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: safeguard against statement errors when requesting a transaction #800

Merged
merged 1 commit into from Jan 14, 2021

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Spanner: TransactionContext hangs thread indefinitely
3 participants