File tree

2 files changed

+93
-17
lines changed

2 files changed

+93
-17
lines changed
Original file line numberDiff line numberDiff line change
@@ -1250,25 +1250,27 @@ public void prepareReadWriteTransaction() {
12501250

12511251
@Override
12521252
public void close() {
1253-
synchronized (lock) {
1254-
Exception = null;
1255-
checkedOutSessions.remove(this);
1256-
}
1257-
PooledSession delegate = getOrNull();
1258-
if (delegate != null) {
1259-
delegate.close();
1253+
try {
1254+
asyncClose().get();
1255+
} catch (InterruptedException e) {
1256+
throw SpannerExceptionFactory.propagateInterrupt(e);
1257+
} catch (ExecutionException e) {
1258+
throw SpannerExceptionFactory.asSpannerException(e.getCause());
12601259
}
12611260
}
12621261

12631262
@Override
12641263
public ApiFuture<Empty> asyncClose() {
1265-
synchronized (lock) {
1266-
Exception = null;
1267-
checkedOutSessions.remove(this);
1268-
}
1269-
PooledSession delegate = getOrNull();
1270-
if (delegate != null) {
1271-
return delegate.asyncClose();
1264+
try {
1265+
PooledSession delegate = getOrNull();
1266+
if (delegate != null) {
1267+
return delegate.asyncClose();
1268+
}
1269+
} finally {
1270+
synchronized (lock) {
1271+
Exception = null;
1272+
checkedOutSessions.remove(this);
1273+
}
12721274
}
12731275
return ApiFutures.immediateFuture(Empty.getDefaultInstance());
12741276
}
@@ -1777,7 +1779,8 @@ private enum Position {
17771779
private final Set<PooledSession> allSessions = new HashSet<>();
17781780

17791781
@GuardedBy("lock")
1780-
private final Set<PooledSessionFuture> checkedOutSessions = new HashSet<>();
1782+
@VisibleForTesting
1783+
final Set<PooledSessionFuture> checkedOutSessions = new HashSet<>();
17811784

17821785
private final SessionConsumer sessionConsumer = new SessionConsumerImpl();
17831786

Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@
6969
import java.util.ArrayList;
7070
import java.util.Collections;
7171
import java.util.List;
72+
import java.util.Set;
7273
import java.util.concurrent.ExecutionException;
7374
import java.util.concurrent.ExecutorService;
7475
import java.util.concurrent.Executors;
@@ -534,13 +535,18 @@ public void testAsyncTransactionManagerCommitWithTag() {
534535

535536
@Test
536537
public void singleUse() {
537-
DatabaseClient client =
538-
spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE));
538+
DatabaseClientImpl client =
539+
(DatabaseClientImpl)
540+
spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE));
541+
Set<PooledSessionFuture> checkedOut = client.pool.checkedOutSessions;
542+
assertThat(checkedOut).isEmpty();
539543
try (ResultSet rs = client.singleUse().executeQuery(SELECT1)) {
540544
assertThat(rs.next()).isTrue();
545+
assertThat(checkedOut).hasSize(1);
541546
assertThat(rs.getLong(0)).isEqualTo(1L);
542547
assertThat(rs.next()).isFalse();
543548
}
549+
assertThat(checkedOut).isEmpty();
544550
}
545551

546552
@Test
@@ -2097,4 +2103,71 @@ public void testAsyncTransactionManagerCommitWithPriority() {
20972103
assertNotNull(request.getRequestOptions());
20982104
assertEquals(Priority.PRIORITY_HIGH, request.getRequestOptions().getPriority());
20992105
}
2106+
2107+
@Test
2108+
public void singleUseNoAction_ClearsCheckedOutSession() {
2109+
DatabaseClientImpl client =
2110+
(DatabaseClientImpl)
2111+
spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE));
2112+
Set<PooledSessionFuture> checkedOut = client.pool.checkedOutSessions;
2113+
assertThat(checkedOut).isEmpty();
2114+
2115+
// Getting a single use read-only transaction and not using it should not cause any sessions
2116+
// to be stuck in the map of checked out sessions.
2117+
client.singleUse().close();
2118+
2119+
assertThat(checkedOut).isEmpty();
2120+
}
2121+
2122+
@Test
2123+
public void singleUseReadOnlyTransactionNoAction_ClearsCheckedOutSession() {
2124+
DatabaseClientImpl client =
2125+
(DatabaseClientImpl)
2126+
spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE));
2127+
Set<PooledSessionFuture> checkedOut = client.pool.checkedOutSessions;
2128+
assertThat(checkedOut).isEmpty();
2129+
2130+
client.singleUseReadOnlyTransaction().close();
2131+
2132+
assertThat(checkedOut).isEmpty();
2133+
}
2134+
2135+
@Test
2136+
public void readWriteTransactionNoAction_ClearsCheckedOutSession() {
2137+
DatabaseClientImpl client =
2138+
(DatabaseClientImpl)
2139+
spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE));
2140+
Set<PooledSessionFuture> checkedOut = client.pool.checkedOutSessions;
2141+
assertThat(checkedOut).isEmpty();
2142+
2143+
client.readWriteTransaction();
2144+
2145+
assertThat(checkedOut).isEmpty();
2146+
}
2147+
2148+
@Test
2149+
public void readOnlyTransactionNoAction_ClearsCheckedOutSession() {
2150+
DatabaseClientImpl client =
2151+
(DatabaseClientImpl)
2152+
spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE));
2153+
Set<PooledSessionFuture> checkedOut = client.pool.checkedOutSessions;
2154+
assertThat(checkedOut).isEmpty();
2155+
2156+
client.readOnlyTransaction().close();
2157+
2158+
assertThat(checkedOut).isEmpty();
2159+
}
2160+
2161+
@Test
2162+
public void transactionManagerNoAction_ClearsCheckedOutSession() {
2163+
DatabaseClientImpl client =
2164+
(DatabaseClientImpl)
2165+
spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE));
2166+
Set<PooledSessionFuture> checkedOut = client.pool.checkedOutSessions;
2167+
assertThat(checkedOut).isEmpty();
2168+
2169+
client.transactionManager().close();
2170+
2171+
assertThat(checkedOut).isEmpty();
2172+
}
21002173
}

0 commit comments

Comments
 (0)