Skip to content

Commit 42840b2

Browse files
author
Arpan Mishra
authored
refactor: remove code related to prepareReadWriteTransactions. (#3023)
* refactor: remove code related to prepareReadWriteTransactions. * chore: remove beginTransactionAsync method.
1 parent 0834406 commit 42840b2

7 files changed

Lines changed: 8 additions & 110 deletions

File tree

java-spanner/google-cloud-spanner/clirr-ignored-differences.xml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,12 @@
2020
<differenceType>8001</differenceType>
2121
<className>com/google/cloud/spanner/connection/StatementParser</className>
2222
</difference>
23+
<!-- (Remove since prepareReadWriteTransaction() has no usage. Session interface is annotated as internal API) -->
24+
<difference>
25+
<differenceType>7002</differenceType>
26+
<className>com/google/cloud/spanner/Session</className>
27+
<method>void prepareReadWriteTransaction()</method>
28+
</difference>
2329
<difference>
2430
<differenceType>7002</differenceType>
2531
<className>com/google/cloud/spanner/SpannerOptions</className>

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

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
import com.google.api.core.ApiFuture;
2020
import com.google.api.core.InternalApi;
21-
import com.google.cloud.spanner.Options.TransactionOption;
2221
import com.google.protobuf.Empty;
2322

2423
/**
@@ -48,16 +47,6 @@ public interface Session extends DatabaseClient, AutoCloseable {
4847
/** Returns the resource name associated with this session. */
4948
String getName();
5049

51-
/**
52-
* Prepares a transaction for use by a subsequent {@link
53-
* DatabaseClient#readWriteTransaction(TransactionOption...)} or {@link #write(Iterable)} call. It
54-
* is not necessary to call this method before running a transaction or performing a write, but
55-
* doing so may allow one round trip of the protocol to be performed in advance; calling this
56-
* method on an idle session that is expected to execute a transaction or write in the near future
57-
* may reduce the latency of the subsequent transaction/write.
58-
*/
59-
void prepareReadWriteTransaction();
60-
6150
@Override
6251
void close();
6352

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

Lines changed: 1 addition & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,6 @@ interface SessionTransaction {
100100
private final String name;
101101
private final DatabaseId databaseId;
102102
private SessionTransaction activeTransaction;
103-
ByteString readyTransactionId;
104103
private final Map<SpannerRpc.Option, ?> options;
105104
private volatile Instant lastUseTime;
106105
@Nullable private final Instant createTime;
@@ -378,12 +377,6 @@ public AsyncTransactionManagerImpl transactionManagerAsync(TransactionOption...
378377
return new AsyncTransactionManagerImpl(this, currentSpan, options);
379378
}
380379

381-
@Override
382-
public void prepareReadWriteTransaction() {
383-
setActive(null);
384-
readyTransactionId = beginTransaction(true);
385-
}
386-
387380
@Override
388381
public ApiFuture<Empty> asyncClose() {
389382
return spanner.getRpc().asyncDeleteSession(name, options);
@@ -402,20 +395,6 @@ public void close() {
402395
}
403396
}
404397

405-
ByteString beginTransaction(boolean routeToLeader) {
406-
try {
407-
return beginTransactionAsync(routeToLeader).get();
408-
} catch (ExecutionException e) {
409-
throw SpannerExceptionFactory.newSpannerException(e.getCause() == null ? e : e.getCause());
410-
} catch (InterruptedException e) {
411-
throw SpannerExceptionFactory.propagateInterrupt(e);
412-
}
413-
}
414-
415-
ApiFuture<ByteString> beginTransactionAsync(boolean routeToLeader) {
416-
return beginTransactionAsync(Options.fromTransactionOptions(), routeToLeader);
417-
}
418-
419398
ApiFuture<ByteString> beginTransactionAsync(Options transactionOptions, boolean routeToLeader) {
420399
final SettableApiFuture<ByteString> res = SettableApiFuture.create();
421400
final ISpan span = tracer.spanBuilder(SpannerImpl.BEGIN_TRANSACTION);
@@ -463,7 +442,7 @@ TransactionContextImpl newTransaction(Options options) {
463442
return TransactionContextImpl.newBuilder()
464443
.setSession(this)
465444
.setOptions(options)
466-
.setTransactionId(readyTransactionId)
445+
.setTransactionId(null)
467446
.setOptions(options)
468447
.setTrackTransactionStarter(spanner.getOptions().isTrackTransactionStarter())
469448
.setRpc(spanner.getRpc())
@@ -484,17 +463,12 @@ <T extends SessionTransaction> T setActive(@Nullable T ctx) {
484463
activeTransaction.invalidate();
485464
}
486465
activeTransaction = ctx;
487-
readyTransactionId = null;
488466
if (activeTransaction != null) {
489467
activeTransaction.setSpan(currentSpan);
490468
}
491469
return ctx;
492470
}
493471

494-
boolean hasReadyTransaction() {
495-
return readyTransactionId != null;
496-
}
497-
498472
TraceWrapper getTracer() {
499473
return tracer;
500474
}

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

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1363,11 +1363,6 @@ public String getName() {
13631363
return get().getName();
13641364
}
13651365

1366-
@Override
1367-
public void prepareReadWriteTransaction() {
1368-
get().prepareReadWriteTransaction();
1369-
}
1370-
13711366
@Override
13721367
public void close() {
13731368
try {
@@ -1618,11 +1613,6 @@ public String getName() {
16181613
return get().getName();
16191614
}
16201615

1621-
@Override
1622-
public void prepareReadWriteTransaction() {
1623-
get().prepareReadWriteTransaction();
1624-
}
1625-
16261616
@Override
16271617
public void close() {
16281618
try {
@@ -1901,12 +1891,6 @@ public String getName() {
19011891
return delegate.getName();
19021892
}
19031893

1904-
@Override
1905-
public void prepareReadWriteTransaction() {
1906-
markUsed();
1907-
delegate.prepareReadWriteTransaction();
1908-
}
1909-
19101894
private void keepAlive() {
19111895
markUsed();
19121896
final ISpan previousSpan = delegate.getCurrentSpan();
@@ -2129,12 +2113,6 @@ public String getName() {
21292113
return delegate.getName();
21302114
}
21312115

2132-
@Override
2133-
public void prepareReadWriteTransaction() {
2134-
throw SpannerExceptionFactory.newSpannerException(
2135-
ErrorCode.UNIMPLEMENTED, "Unimplemented with Multiplexed Session");
2136-
}
2137-
21382116
@Override
21392117
public void close() {
21402118
synchronized (lock) {

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

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -375,20 +375,6 @@ public void singleUseContextClosesTransaction() {
375375
assertThat(e.getMessage()).contains("invalidated");
376376
}
377377

378-
@Test
379-
public void prepareClosesOldSingleUseContext() {
380-
ReadContext ctx = session.singleUse(TimestampBound.strong());
381-
382-
Mockito.when(rpc.beginTransaction(Mockito.any(), Mockito.eq(options), eq(false)))
383-
.thenReturn(Transaction.newBuilder().setId(ByteString.copyFromUtf8("t1")).build());
384-
session.prepareReadWriteTransaction();
385-
IllegalStateException e =
386-
assertThrows(
387-
IllegalStateException.class,
388-
() -> ctx.read("Dummy", KeySet.all(), Collections.singletonList("C")));
389-
assertThat(e.getMessage()).contains("invalidated");
390-
}
391-
392378
private static ResultSetMetadata newMetadata(Type type) {
393379
return ResultSetMetadata.newBuilder().setRowType(type.toProto().getStructType()).build();
394380
}

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

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
import com.google.cloud.spanner.SessionPoolOptions.InactiveTransactionRemovalOptions;
3333
import com.google.cloud.spanner.spi.v1.SpannerRpc.Option;
3434
import com.google.common.util.concurrent.Uninterruptibles;
35-
import com.google.protobuf.ByteString;
3635
import com.google.protobuf.Empty;
3736
import io.opencensus.trace.Tracing;
3837
import io.opentelemetry.api.OpenTelemetry;
@@ -161,21 +160,6 @@ public ApiFuture<Empty> asyncClose() {
161160
}
162161
return ApiFutures.immediateFuture(Empty.getDefaultInstance());
163162
}
164-
165-
@Override
166-
public void prepareReadWriteTransaction() {
167-
if (random.nextInt(100) < 10) {
168-
expireSession(this);
169-
throw SpannerExceptionFactoryTest.newSessionNotFoundException(this.getName());
170-
}
171-
String name = this.getName();
172-
synchronized (lock) {
173-
if (sessions.put(name, true)) {
174-
setFailed();
175-
}
176-
this.readyTransactionId = ByteString.copyFromUtf8("foo");
177-
}
178-
}
179163
};
180164
sessionIndex++;
181165
return session;
@@ -192,18 +176,9 @@ private void setupSession(final SessionImpl session, final ReadContext mockConte
192176
when(mockResult.next()).thenReturn(true);
193177
}
194178

195-
private void expireSession(Session session) {
196-
String name = session.getName();
197-
synchronized (lock) {
198-
sessions.remove(name);
199-
expiredSessions.add(name);
200-
}
201-
}
202-
203179
private void resetTransaction(SessionImpl session) {
204180
String name = session.getName();
205181
synchronized (lock) {
206-
session.readyTransactionId = null;
207182
sessions.put(name, false);
208183
}
209184
}

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

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -304,14 +304,7 @@ public void inlineBegin() {
304304

305305
SessionImpl session =
306306
new SessionImpl(
307-
spanner, "projects/p/instances/i/databases/d/sessions/s", Collections.EMPTY_MAP) {
308-
@Override
309-
public void prepareReadWriteTransaction() {
310-
// Using a prepared transaction is not allowed when the beginTransaction should be
311-
// inlined with the first statement.
312-
throw new IllegalStateException();
313-
}
314-
};
307+
spanner, "projects/p/instances/i/databases/d/sessions/s", Collections.EMPTY_MAP) {};
315308
session.setCurrentSpan(new OpenTelemetrySpan(mock(io.opentelemetry.api.trace.Span.class)));
316309
TransactionRunnerImpl runner = new TransactionRunnerImpl(session);
317310
runner.setSpan(span);
@@ -342,9 +335,6 @@ private long[] batchDmlException(int status) {
342335
.setSpan(session.getTracer().getCurrentSpan())
343336
.build();
344337
when(session.newTransaction(Options.fromTransactionOptions())).thenReturn(transaction);
345-
when(session.beginTransactionAsync(true))
346-
.thenReturn(
347-
ApiFutures.immediateFuture(ByteString.copyFromUtf8(UUID.randomUUID().toString())));
348338
when(session.getName()).thenReturn(SessionId.of("p", "i", "d", "test").getName());
349339
TransactionRunnerImpl runner = new TransactionRunnerImpl(session);
350340
runner.setSpan(span);

0 commit comments

Comments
 (0)