Skip to content
Permalink
Browse files
fix: session retry could cause infinite wait (#616)
A "Session not found" when using an AsyncTransactionManager could cause an
infinite wait for an ApiFuture that would never be done.

Fixes #605
  • Loading branch information
olavloite committed Nov 13, 2020
1 parent b85be2a commit 8a66d84edbdaeba6b021d962a9b1984a3d2f40df
@@ -98,6 +98,7 @@ private ApiFuture<TransactionContext> internalBeginAsync(boolean firstAttempt) {
new ApiFutureCallback<Void>() {
@Override
public void onFailure(Throwable t) {
onError(t);

Check warning on line 101 in google-cloud-spanner/src/main/java/com/google/cloud/spanner/AsyncTransactionManagerImpl.java

Codecov / codecov/patch

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

Added line #L101 was not covered by tests
res.setException(SpannerExceptionFactory.newSpannerException(t));
}

@@ -130,6 +131,7 @@ public ApiFuture<Timestamp> commitAsync() {
}
ApiFuture<Timestamp> res = txn.commitAsync();
txnState = TransactionState.COMMITTED;

ApiFutures.addCallback(
res,
new ApiFutureCallback<Timestamp>() {
@@ -174,10 +176,6 @@ public ApiFuture<Void> apply(Empty input) throws Exception {

@Override
public TransactionContextFuture resetForRetryAsync() {
if (txn == null || (!txn.isAborted() && txnState != TransactionState.ABORTED)) {
throw new IllegalStateException(
"resetForRetry can only be called if the previous attempt aborted");
}
return new TransactionContextFutureImpl(this, internalBeginAsync(false));
}

@@ -38,6 +38,9 @@
@GuardedBy("lock")
private TransactionState txnState;

@GuardedBy("lock")
private AbortedException abortedException;

private final SessionPool pool;
private volatile PooledSessionFuture session;
private volatile SettableApiFuture<AsyncTransactionManagerImpl> delegate;
@@ -159,6 +162,7 @@ public void onError(Throwable t) {
if (t instanceof AbortedException) {
synchronized (lock) {
txnState = TransactionState.ABORTED;
abortedException = (AbortedException) t;
}
}
}
@@ -167,9 +171,12 @@ public void onError(Throwable t) {
public ApiFuture<Timestamp> commitAsync() {
synchronized (lock) {
Preconditions.checkState(
txnState == TransactionState.STARTED,
txnState == TransactionState.STARTED || txnState == TransactionState.ABORTED,
"commit can only be invoked if the transaction is in progress. Current state: "
+ txnState);
if (txnState == TransactionState.ABORTED) {
return ApiFutures.immediateFailedFuture(abortedException);

Check warning on line 178 in google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolAsyncTransactionManager.java

Codecov / codecov/patch

google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolAsyncTransactionManager.java#L178

Added line #L178 was not covered by tests
}
txnState = TransactionState.COMMITTED;
}
return ApiFutures.transformAsync(
@@ -186,6 +193,7 @@ public void onFailure(Throwable t) {
synchronized (lock) {
if (t instanceof AbortedException) {
txnState = TransactionState.ABORTED;
abortedException = (AbortedException) t;
} else {
txnState = TransactionState.COMMIT_FAILED;
}
@@ -109,6 +109,7 @@ public Timestamp get(long timeout, TimeUnit unit)
@Override
public void onFailure(Throwable t) {
mgr.onError(t);
statementResult.setException(t);
txnResult.setException(t);
}

0 comments on commit 8a66d84

Please sign in to comment.