Skip to content

Commit e8cff3f

Browse files
nithinsujirpongad
nithinsujir
authored and
pongad
committed
Spanner: Block nested transactions (#3628)
Cloud spanner does not support nested transactions. Use a thread-local flag to check and throw exception. Signed-off-by: Nithin Nayak Sujir <[email protected]>
1 parent 7869364 commit e8cff3f

File tree

6 files changed

+184
-2
lines changed

6 files changed

+184
-2
lines changed

‎google-cloud-clients/google-cloud-spanner/pom.xml

+2
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
</parent>
1919
<properties>
2020
<site.installationModule>google-cloud-spanner</site.installationModule>
21+
<skipUnitTests>${skipTests}</skipUnitTests>
2122
<jacoco.skip>true</jacoco.skip>
2223
</properties>
2324
<build>
@@ -47,6 +48,7 @@
4748
<version>2.12.4</version>
4849
<configuration>
4950
<excludedGroups>com.google.cloud.spanner.IntegrationTest</excludedGroups>
51+
<skipTests>${skipUnitTests}</skipTests>
5052
<reportNameSuffix>sponge_log</reportNameSuffix>
5153
</configuration>
5254
</plugin>

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

+6
Original file line numberDiff line numberDiff line change
@@ -408,6 +408,12 @@ public <T> T run(TransactionCallable<T> callable) {
408408
public Timestamp getCommitTimestamp() {
409409
return runner.getCommitTimestamp();
410410
}
411+
412+
@Override
413+
public TransactionRunner allowNestedTransaction() {
414+
runner.allowNestedTransaction();
415+
return runner;
416+
}
411417
};
412418
}
413419

‎google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java

+31-1
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,6 @@
8181
import io.opencensus.trace.Span;
8282
import io.opencensus.trace.Tracer;
8383
import io.opencensus.trace.Tracing;
84-
8584
import java.io.IOException;
8685
import java.io.Serializable;
8786
import java.util.AbstractList;
@@ -129,6 +128,19 @@ class SpannerImpl extends BaseService<SpannerOptions> implements Spanner {
129128
private static final String QUERY = "CloudSpannerOperation.ExecuteStreamingQuery";
130129
private static final String READ = "CloudSpannerOperation.ExecuteStreamingRead";
131130

131+
private static final ThreadLocal<Boolean> hasPendingTransaction = new ThreadLocal<Boolean>() {
132+
@Override
133+
protected Boolean initialValue() {
134+
return false;
135+
}
136+
};
137+
138+
private static void throwIfTransactionsPending() {
139+
if (hasPendingTransaction.get() == Boolean.TRUE) {
140+
throw newSpannerException(ErrorCode.INTERNAL, "Nested transactions are not supported");
141+
}
142+
}
143+
132144
static {
133145
TraceUtil.exportSpans(CREATE_SESSION, DELETE_SESSION, BEGIN_TRANSACTION, COMMIT, QUERY, READ);
134146
}
@@ -905,6 +917,8 @@ TransactionContextImpl newTransaction() {
905917
}
906918

907919
<T extends SessionTransaction> T setActive(@Nullable T ctx) {
920+
throwIfTransactionsPending();
921+
908922
if (activeTransaction != null) {
909923
activeTransaction.invalidate();
910924
}
@@ -1209,6 +1223,7 @@ public void execute(Runnable command) {
12091223

12101224
@VisibleForTesting
12111225
static class TransactionRunnerImpl implements SessionTransaction, TransactionRunner {
1226+
private boolean blockNestedTxn = true;
12121227

12131228
/** Allow for testing of backoff logic */
12141229
static class Sleeper {
@@ -1223,6 +1238,11 @@ void backoffSleep(Context context, long backoffMillis) {
12231238
private TransactionContextImpl txn;
12241239
private volatile boolean isValid = true;
12251240

1241+
public TransactionRunner allowNestedTransaction() {
1242+
blockNestedTxn = false;
1243+
return this;
1244+
}
1245+
12261246
TransactionRunnerImpl(
12271247
SessionImpl session, SpannerRpc rpc, Sleeper sleeper, int defaultPrefetchChunks) {
12281248
this.session = session;
@@ -1239,11 +1259,19 @@ void backoffSleep(Context context, long backoffMillis) {
12391259
@Override
12401260
public <T> T run(TransactionCallable<T> callable) {
12411261
try (Scope s = tracer.withSpan(span)) {
1262+
if (blockNestedTxn) {
1263+
hasPendingTransaction.set(Boolean.TRUE);
1264+
}
1265+
12421266
return runInternal(callable);
12431267
} catch (RuntimeException e) {
12441268
TraceUtil.endSpanWithFailure(span, e);
12451269
throw e;
12461270
} finally {
1271+
// Remove threadLocal rather than set to FALSE to avoid a possible memory leak.
1272+
// We also do this unconditionally in case a user has modified the flag when the transaction
1273+
// was running.
1274+
hasPendingTransaction.remove();
12471275
span.end();
12481276
}
12491277
}
@@ -1660,6 +1688,8 @@ ByteString getTransactionId() {
16601688
}
16611689

16621690
void initTransaction() {
1691+
throwIfTransactionsPending();
1692+
16631693
// Since we only support synchronous calls, just block on "txnLock" while the RPC is in
16641694
// flight. Note that we use the strategy of sending an explicit BeginTransaction() RPC,
16651695
// rather than using the first read in the transaction to begin it implicitly. The chosen

‎google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunner.java

+19
Original file line numberDiff line numberDiff line change
@@ -73,4 +73,23 @@ interface TransactionCallable<T> {
7373
* {@link #run(TransactionCallable)} has returned normally.
7474
*/
7575
Timestamp getCommitTimestamp();
76+
77+
/**
78+
* Allows overriding the default behaviour of blocking nested transactions.
79+
*
80+
* Note that the client library does not maintain any information regarding the nesting structure.
81+
* If an outer transaction fails and an inner transaction succeeds, upon retry of the outer
82+
* transaction, the inner transaction will be re-executed.
83+
*
84+
* Use with care when certain that the inner transaction is idempotent. Avoid doing this when
85+
* accessing the same db. There might be legitimate uses where access need to be made across DBs
86+
* for instance.
87+
*
88+
* E.g. of nesting that is discouraged, see {@code nestedReadWriteTxnThrows}
89+
* {@code nestedReadOnlyTxnThrows}, {@code nestedBatchTxnThrows},
90+
* {@code nestedSingleUseReadTxnThrows}
91+
*
92+
* @return this object
93+
*/
94+
TransactionRunner allowNestedTransaction();
7695
}

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

-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
import io.grpc.Status;
3333
import io.grpc.StatusRuntimeException;
3434
import java.util.concurrent.atomic.AtomicInteger;
35-
3635
import org.junit.Before;
3736
import org.junit.Test;
3837
import org.junit.runner.RunWith;

‎google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITTransactionTest.java

+126
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,17 @@
2323

2424
import com.google.cloud.Timestamp;
2525
import com.google.cloud.spanner.AbortedException;
26+
import com.google.cloud.spanner.BatchClient;
27+
import com.google.cloud.spanner.BatchReadOnlyTransaction;
2628
import com.google.cloud.spanner.Database;
2729
import com.google.cloud.spanner.DatabaseClient;
2830
import com.google.cloud.spanner.ErrorCode;
2931
import com.google.cloud.spanner.IntegrationTest;
3032
import com.google.cloud.spanner.IntegrationTestEnv;
3133
import com.google.cloud.spanner.Key;
34+
import com.google.cloud.spanner.KeySet;
3235
import com.google.cloud.spanner.Mutation;
36+
import com.google.cloud.spanner.PartitionOptions;
3337
import com.google.cloud.spanner.ReadContext;
3438
import com.google.cloud.spanner.ResultSet;
3539
import com.google.cloud.spanner.SpannerException;
@@ -349,4 +353,126 @@ public Void run(TransactionContext transaction) throws SpannerException {
349353
.getLong(0))
350354
.isEqualTo(2);
351355
}
356+
357+
private void doNestedRwTransaction() {
358+
client
359+
.readWriteTransaction()
360+
.run(
361+
new TransactionCallable<Void>() {
362+
@Override
363+
public Void run(TransactionContext transaction) throws SpannerException {
364+
client
365+
.readWriteTransaction()
366+
.run(
367+
new TransactionCallable<Void>() {
368+
@Override
369+
public Void run(TransactionContext transaction) throws Exception {
370+
return null;
371+
}
372+
});
373+
374+
return null;
375+
}
376+
});
377+
}
378+
379+
@Test
380+
public void nestedReadWriteTxnThrows() {
381+
try {
382+
doNestedRwTransaction();
383+
fail("Expected exception");
384+
} catch (SpannerException e) {
385+
assertThat(e.getErrorCode()).isEqualTo(ErrorCode.INTERNAL);
386+
assertThat(e.getMessage()).contains("not supported");
387+
}
388+
}
389+
390+
@Test
391+
public void nestedReadOnlyTxnThrows() {
392+
try {
393+
client
394+
.readWriteTransaction()
395+
.run(
396+
new TransactionCallable<Void>() {
397+
@Override
398+
public Void run(TransactionContext transaction) throws SpannerException {
399+
client
400+
.readOnlyTransaction()
401+
.getReadTimestamp();
402+
403+
return null;
404+
}
405+
});
406+
fail("Expected exception");
407+
} catch (SpannerException e) {
408+
assertThat(e.getErrorCode()).isEqualTo(ErrorCode.INTERNAL);
409+
assertThat(e.getMessage()).contains("not supported");
410+
}
411+
}
412+
413+
@Test
414+
public void nestedBatchTxnThrows() {
415+
try {
416+
client
417+
.readWriteTransaction()
418+
.run(
419+
new TransactionCallable<Void>() {
420+
@Override
421+
public Void run(TransactionContext transaction) throws SpannerException {
422+
BatchClient batchClient = env.getTestHelper().getBatchClient(db);
423+
BatchReadOnlyTransaction batchTxn = batchClient
424+
.batchReadOnlyTransaction(TimestampBound.strong());
425+
batchTxn.partitionReadUsingIndex(
426+
PartitionOptions.getDefaultInstance(),
427+
"Test",
428+
"Index",
429+
KeySet.all(),
430+
Arrays.asList("Fingerprint"));
431+
432+
return null;
433+
}
434+
});
435+
fail("Expected exception");
436+
} catch (SpannerException e) {
437+
assertThat(e.getErrorCode()).isEqualTo(ErrorCode.INTERNAL);
438+
assertThat(e.getMessage()).contains("not supported");
439+
}
440+
}
441+
442+
@Test
443+
public void nestedSingleUseReadTxnThrows() {
444+
try {
445+
client
446+
.readWriteTransaction()
447+
.run(
448+
new TransactionCallable<Void>() {
449+
@Override
450+
public Void run(TransactionContext transaction) throws SpannerException {
451+
client.singleUseReadOnlyTransaction();
452+
453+
return null;
454+
}
455+
});
456+
fail("Expected exception");
457+
} catch (SpannerException e) {
458+
assertThat(e.getErrorCode()).isEqualTo(ErrorCode.INTERNAL);
459+
assertThat(e.getMessage()).contains("not supported");
460+
}
461+
}
462+
463+
@Test
464+
public void nestedTxnSucceedsWhenAllowed() {
465+
client
466+
.readWriteTransaction()
467+
.allowNestedTransaction()
468+
.run(
469+
new TransactionCallable<Void>() {
470+
@Override
471+
public Void run(TransactionContext transaction) throws SpannerException {
472+
client.singleUseReadOnlyTransaction();
473+
474+
return null;
475+
}
476+
});
477+
}
352478
}

0 commit comments

Comments
 (0)