Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
A "Session not found" when using an AsyncTransactionManager could cause an infinite wait for an ApiFuture that would never be done. Fixes #605
olavloite commented Nov 11, 2020
| @@ -109,6 +109,7 @@ public Timestamp get(long timeout, TimeUnit unit) | |||
| @Override | |||
| public void onFailure(Throwable t) { | |||
| mgr.onError(t); | |||
| statementResult.setException(t); | |||
There was a problem hiding this comment.
This missing statement was what could cause the infinite wait. The result of this specific statement would never be done if the previous statement (input) would fail.
olavloite commented Nov 11, 2020
| @@ -174,10 +176,6 @@ public void onSuccess(Timestamp result) { | |||
| @Override | |||
| public TransactionContextFuture resetForRetryAsync() { | |||
| if (txn == null || (!txn.isAborted() && txnState != TransactionState.ABORTED)) { | |||
There was a problem hiding this comment.
This check is no longer needed here as it is checked in the SessionPoolAsyncTransactionManager.
olavloite commented Nov 11, 2020
| "commit can only be invoked if the transaction is in progress. Current state: " | ||
| + txnState); | ||
| if (txnState == TransactionState.ABORTED) { |
There was a problem hiding this comment.
This makes sure that the AsyncTransactionManager will return an ApiFuture with exception Aborted if the transaction has already been aborted by an asynchronous statement that has not yet been checked by the application, e.g. (simplified):
txn.updateAsync(...); // This aborts the transaction, but the status of the returned ApiFuture is not checked by the application.
...
txn.commitAsync();Codecov Report
@@ Coverage Diff @@
## master #616 +/- ##
============================================
+ Coverage 83.79% 84.04% +0.24%
- Complexity 2500 2502 +2
============================================
Files 141 141
Lines 13770 13810 +40
Branches 1278 1318 +40
============================================
+ Hits 11538 11606 +68
- Misses 1655 1657 +2
+ Partials 577 547 -30
Continue to review full report at Codecov.
|
thiagotnunes approved these changes Nov 13, 2020
rajatbhatta pushed a commit to rajatbhatta/java-spanner that referenced this pull request Nov 17, 2022
…ts (googleapis#616) Removes the category from the base class for all integration tests so they are not excluded from multiple builds, as the result is that both the category of the base class as well as the concrete class are applied.
Sign up for free to join this conversation on . Already have an account? Sign in to comment
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
A "Session not found" when using an AsyncTransactionManager could cause an infinite wait for an
ApiFuturethat would never be done.Fixes #605