Skip to content

Commit d9813a0

Browse files
authored
fix: AsyncTransactionManager did not always close the session (#3580)
In a case where the following happens, the session that was used by an AsyncTransactionManager would not be returned to the pool: 1. The transaction is abandoned without a Commit or Rollback call. 2. The AsyncTransactionManager then executes a RollbackAsync internally. 3. If the internal RollbackAsync failed, then the session would not be closed.
1 parent c74fe83 commit d9813a0

File tree

2 files changed

+33
-0
lines changed

2 files changed

+33
-0
lines changed

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

+1
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ public void onSuccess(AsyncTransactionManagerImpl result) {
107107
new ApiFutureCallback<Void>() {
108108
@Override
109109
public void onFailure(Throwable t) {
110+
session.close();
110111
res.setException(t);
111112
}
112113

google-cloud-spanner/src/test/java/com/google/cloud/spanner/AsyncTransactionManagerTest.java

+32
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import static org.junit.Assert.assertEquals;
2828
import static org.junit.Assert.assertNotNull;
2929
import static org.junit.Assert.assertThrows;
30+
import static org.junit.Assert.assertTrue;
3031

3132
import com.google.api.core.ApiFuture;
3233
import com.google.api.core.ApiFutureCallback;
@@ -1084,6 +1085,37 @@ public void onSuccess(Long aLong) {
10841085
}
10851086
}
10861087

1088+
@Test
1089+
public void testAbandonedAsyncTransactionManager_rollbackFails() throws Exception {
1090+
mockSpanner.setRollbackExecutionTime(
1091+
SimulatedExecutionTime.ofException(Status.PERMISSION_DENIED.asRuntimeException()));
1092+
1093+
boolean gotException = false;
1094+
try (AsyncTransactionManager manager = client().transactionManagerAsync()) {
1095+
TransactionContextFuture transactionContextFuture = manager.beginAsync();
1096+
while (true) {
1097+
try {
1098+
AsyncTransactionStep<Void, Long> updateCount =
1099+
transactionContextFuture.then(
1100+
(transactionContext, ignored) ->
1101+
transactionContext.executeUpdateAsync(UPDATE_STATEMENT),
1102+
executor);
1103+
assertEquals(1L, updateCount.get().longValue());
1104+
// Break without committing or rolling back the transaction.
1105+
break;
1106+
} catch (AbortedException e) {
1107+
transactionContextFuture = manager.resetForRetryAsync();
1108+
}
1109+
}
1110+
} catch (SpannerException spannerException) {
1111+
// The error from the automatically executed Rollback is surfaced when the
1112+
// AsyncTransactionManager is closed.
1113+
assertEquals(ErrorCode.PERMISSION_DENIED, spannerException.getErrorCode());
1114+
gotException = true;
1115+
}
1116+
assertTrue(gotException);
1117+
}
1118+
10871119
private boolean isMultiplexedSessionsEnabled() {
10881120
if (spanner.getOptions() == null || spanner.getOptions().getSessionPoolOptions() == null) {
10891121
return false;

0 commit comments

Comments
 (0)