Skip to content

Commit 3261ed6

Browse files
committed
chore(spanner): review comments
1 parent 3553da3 commit 3261ed6

File tree

5 files changed

+24
-13
lines changed

5 files changed

+24
-13
lines changed

google-cloud-spanner/src/main/java/com/google/cloud/spanner/AsyncTransactionManager.java

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -171,13 +171,15 @@ interface AsyncTransactionFunction<I, O> {
171171
TransactionContextFuture beginAsync();
172172

173173
/**
174-
* Initializes a new read-write transaction. This method must be called before performing any
175-
* operations, and it can only be invoked once per transaction lifecycle.
174+
* Initializes a new read-write transaction that is a retry of a previously aborted transaction.
175+
* This method must be called before performing any operations, and it can only be invoked once
176+
* per transaction lifecycle.
176177
*
177-
* <p>This is especially useful in scenarios involving multiplexed sessions and when creating a
178-
* new transaction for retry attempts. If {@link #resetForRetryAsync()} is not used, you can pass
179-
* the {@link AbortedException} from a previous attempt here to preserve the transaction's
180-
* priority.
178+
* <p>This method should only be used when multiplexed sessions are enabled to create a retry for
179+
* a previously aborted transaction. This method can be used instead of {@link
180+
* #resetForRetryAsync()} to create a retry. Using this method or {@link #resetForRetryAsync()}
181+
* will have the same effect. You must pass in the {@link AbortedException} from the previous
182+
* attempt to preserve the transaction's priority.
181183
*
182184
* <p>For regular sessions, this behaves the same as {@link #beginAsync()}.
183185
*/

google-cloud-spanner/src/main/java/com/google/cloud/spanner/AsyncTransactionManagerImpl.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ public TransactionContextFutureImpl beginAsync() {
8282
@Override
8383
public TransactionContextFutureImpl beginAsync(AbortedException exception) {
8484
Preconditions.checkState(txn == null, "begin can only be called once");
85+
Preconditions.checkNotNull(exception, "AbortedException from the previous attempt is required");
8586
ByteString abortedTransactionId =
8687
exception.getTransactionID() != null ? exception.getTransactionID() : ByteString.EMPTY;
8788
return new TransactionContextFutureImpl(this, internalBeginAsync(true, abortedTransactionId));

google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionManager.java

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,12 +62,15 @@ enum TransactionState {
6262
TransactionContext begin();
6363

6464
/**
65-
* Initializes a new read-write transaction. This method must be called before performing any
66-
* operations, and it can only be invoked once per transaction lifecycle.
65+
* Initializes a new read-write transaction that is a retry of a previously aborted transaction.
66+
* This method must be called before performing any operations, and it can only be invoked once
67+
* per transaction lifecycle.
6768
*
68-
* <p>This is especially useful in scenarios involving multiplexed sessions and when creating a
69-
* new transaction for retry attempts. If {@link #resetForRetry()} is not used, you can pass the
70-
* {@link AbortedException} from a previous attempt here to preserve the transaction's priority.
69+
* <p>This method should only be used when multiplexed sessions are enabled to create a retry for
70+
* a previously aborted transaction. This method can be used instead of {@link #resetForRetry()}
71+
* to create a retry. Using this method or {@link #resetForRetry()} will have the same effect. You
72+
* must pass in the {@link AbortedException} from the previous attempt to preserve the
73+
* transaction's priority.
7174
*
7275
* <p>For regular sessions, this behaves the same as {@link #begin()}.
7376
*/

google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionManagerImpl.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,15 @@ public TransactionContext begin() {
5959
@Override
6060
public TransactionContext begin(AbortedException exception) {
6161
Preconditions.checkState(txn == null, "begin can only be called once");
62+
Preconditions.checkNotNull(exception, "AbortedException from the previous attempt is required");
6263
ByteString previousAbortedTransactionID =
6364
exception.getTransactionID() != null ? exception.getTransactionID() : ByteString.EMPTY;
6465
return begin(previousAbortedTransactionID);
6566
}
6667

6768
TransactionContext begin(ByteString previousTransactionId) {
6869
try (IScope s = tracer.withSpan(span)) {
69-
txn = session.newTransaction(options, /* previousTransactionId = */ previousTransactionId);
70+
txn = session.newTransaction(options, previousTransactionId);
7071
session.setActive(this);
7172
txnState = TransactionState.STARTED;
7273
return txn;

google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -793,7 +793,11 @@ public SpannerException onError(SpannerException e, boolean withBeginTransaction
793793
long delay = -1L;
794794
if (exceptionToThrow instanceof AbortedException) {
795795
delay = exceptionToThrow.getRetryDelayInMillis();
796-
((AbortedException) exceptionToThrow).setTransactionID(this.transactionId);
796+
((AbortedException) exceptionToThrow)
797+
.setTransactionID(
798+
this.transactionId != null
799+
? this.transactionId
800+
: this.getPreviousTransactionId());
797801
}
798802
if (delay == -1L) {
799803
txnLogger.log(

0 commit comments

Comments
 (0)