Conversation

olavloite

If the first statement of a read/write transaction fails with a CANCELLED error and the error message is Read/query was cancelled due to the enclosing transaction being invalidated by a later transaction in the same session., then the transaction should be retried, as the error could be caused by a previous statement that was abandoned by the client but still executed by the backend. This could be the case if the statement timed out (on the client) or was cancelled.

Fixes #938

If the first statement of a read/write transaction fails with a CANCELLED error
and the error message is `Read/query was cancelled due to the enclosing transaction
being invalidated by a later transaction in the same session.`, then the transaction
should be retried, as the error could be caused by a previous statement that was
abandoned by the client but still executed by the backend. This could be the case
if the statement timed out (on the client) or was cancelled.

Fixes #938
@olavloiteolavloite requested a review from a team as a code owner March 19, 2021 12:04
@google-clagoogle-cla bot added the cla: yesThis human has signed the Contributor License Agreement.label Mar 19, 2021
@product-auto-labelproduct-auto-label bot added the api: spannerIssues related to the googleapis/java-spanner API.label Mar 19, 2021
@codecov

Codecov Report

Merging #999 (3c5b5e1) into master (459a477) will increase coverage by 0.06%.
The diff coverage is 95.65%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #999      +/-   ##
============================================
+ Coverage     85.07%   85.13%   +0.06%     
- Complexity     2617     2621       +4     
============================================
  Files           154      154              
  Lines         14319    14324       +5     
  Branches       1331     1334       +3     
============================================
+ Hits          12182    12195      +13     
+ Misses         1569     1566       -3     
+ Partials        568      563       -5     
Impacted FilesCoverage ΔComplexity Δ
...om/google/cloud/spanner/TransactionRunnerImpl.java86.07% <95.00%> (+0.14%)10.00 <0.00> (ø)
.../com/google/cloud/spanner/AbstractReadContext.java86.54% <100.00%> (ø)42.00 <1.00> (ø)
...va/com/google/cloud/spanner/AbstractResultSet.java83.45% <100.00%> (ø)28.00 <0.00> (ø)
...ain/java/com/google/cloud/spanner/SessionPool.java89.32% <0.00%> (+0.38%)73.00% <0.00%> (+1.00%)
.../google/cloud/spanner/SpannerExceptionFactory.java87.96% <0.00%> (+2.77%)48.00% <0.00%> (+2.00%)
.../google/cloud/spanner/AbstractLazyInitializer.java100.00% <0.00%> (+7.14%)5.00% <0.00%> (+1.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 459a477...3c5b5e1. Read the comment docs.

Choose a reason for hiding this comment

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

@olavloite without this fix, the user would just see a cancelled error and no retry would be made automatically, right?

SpannerException exceptionToThrow;
if (withBeginTransaction
&& e.getErrorCode() == ErrorCode.CANCELLED
&& e.getMessage().contains("invalidated by a later transaction")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: could we extract a constant for this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@olavloite

@olavloite without this fix, the user would just see a cancelled error and no retry would be made automatically, right?

Correct. That is also exactly what happened in #938.

@olavloiteolavloite added the automergeMerge the pull request once unit tests and other checks pass.label Mar 22, 2021
@gcf-merge-on-greengcf-merge-on-green bot merged commit a95f6f8 into master Mar 22, 2021
@gcf-merge-on-greengcf-merge-on-green bot deleted the retry-cancelled-on-first-statement branch March 22, 2021 09:34
@gcf-merge-on-greengcf-merge-on-green bot removed the automergeMerge the pull request once unit tests and other checks pass.label Mar 22, 2021
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.connection.it.ITReadWriteAutocommitSpannerTest: test03_MultipleStatements_WithTimeouts failed