Spanner: Block nested transactions #3628

nithinsujir · 2018-08-31T16:08:08Z

Cloud spanner does not support nested transactions. Use a thread-local
flag to check and throw exception. The flag is set to true when a transaction begins running. The checks are present when the possibly nested transactions are being prepared.

Single and batch transactions have slightly different paths, so the check is needed in two places.

@jcorwin

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]>

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

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

@@ -905,6 +917,8 @@ TransactionContextImpl newTransaction() {
    }

    <T extends SessionTransaction> T setActive(@Nullable T ctx) {
+      throwIfTransactionsPending();


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

+  }
+
+  @Test
+  public void nestedRwRwTransactionShouldThrowException() {


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

@@ -905,6 +917,8 @@ TransactionContextImpl newTransaction() {
    }

    <T extends SessionTransaction> T setActive(@Nullable T ctx) {
+      throwIfTransactionsPending();


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

@@ -73,4 +73,6 @@
   * {@link #run(TransactionCallable)} has returned normally.
   */
  Timestamp getCommitTimestamp();
+
+  TransactionRunner allowNestedTransaction();


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

+  }
+
+  @Test
+  public void nestedRwRwTransactionShouldThrowException() {


…earer

snehashah16

LGTM

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

        return runInternal(callable);
      } catch (RuntimeException e) {
        TraceUtil.endSpanWithFailure(span, e);
        throw e;
      } finally {
+        if (blockNestedTxn) {
+          hasPendingTransaction.set(Boolean.FALSE);


nithinsujir · 2018-09-18T23:06:38Z

@pongad Has my new commit addressed your comments adequately?

pongad · 2018-09-18T23:58:39Z

@nithinsujir Hey sorry. I think the code looks good. There's a merge conflict though. Could you take a look? I think I can merge after that.

nithinsujir · 2018-09-19T00:12:50Z

Resolved the conflict.

- b2056bd147466fec8a817885fc5da0f5f39b5730 Integration tests for VPC-SC. (googleapis#48) by Qian Long <[email protected]> - 8ce495510ff37cb27a1c840d212288cb5c87d327 Import of package:google-cloud-spanner to GitHub Private ... by Cloud Spanner Team <[email protected]> - 8493b86 Re-generate library using google-cloud-clients/google-clo... by DPE bot <[email protected]> - 5555939 Release v0.66.1-SNAPSHOT (googleapis#3784) by Jeff Ching <[email protected]> - ed6a3f5 Release google-cloud-java v0.66.0 (googleapis#3782) by Jeff Ching <[email protected]> - 1e539fa Cloud Spanner DML & PDML Release (googleapis#3781) by snehashah16 <[email protected]> - d197058 add null check to JobInfo.fromPb(Job) and .toPb() (googleapis#3770) by sombra-mykola-bakay <[email protected]> - 211bfad Add synthtool scripts (googleapis#3765) by Jeff Ching <[email protected]> - 4813432 Add listDocuments() API (googleapis#3759) by Sebastian Schmidt <[email protected]> - 0f3bc0e Don't raise empty Snapshots on resets (googleapis#3750) by Sebastian Schmidt <[email protected]> - 63aff86 Remove CircleCI status, coveralls, and version eye badges... by Jeff Ching <[email protected]> - 6a8e5d6 Bump to next snapshot version (googleapis#3768) by Jeff Ching <[email protected]> - 1093938 Release 1.47.0/0.65.0 (googleapis#3767) by Jeff Ching <[email protected]> - f69710d Deprecate getCollections in favor of listCollections (googleapis#37... by Sebastian Schmidt <[email protected]> - 7f74c5a [Storage] Add launch stage annotations around bucket lock... by Frank Natividad <[email protected]> - 152653d Regenerate clients (googleapis#3766) by Jeff Ching <[email protected]> - 68f38e1 Revert "Revert "Revert "Cloud Spanner DML & PartitionedDM... by Jeff Ching <[email protected]> - f741fc0 Fix README typo (googleapis#3762) by ihommani <[email protected]> - 65ca2f3 Revert "Revert "Cloud Spanner DML & PartitionedDML suppor... by Jeff Ching <[email protected]> - 6ebbc19 fix incorrect link (googleapis#3748) by Elliotte Rusty Harold <[email protected]> - bcb28f2 [Storage] Bucket Lock (googleapis#3727) by Frank Natividad <[email protected]> - c79f280 spanner: Expand test coverage for GrpcResultSet and runWi... by Nithin Sujir <[email protected]> - fdccb0b Bump to next snapshot version (googleapis#3744) by JesseLovelace <[email protected]> - 456e8fb Release 1.46.0/0.64.0 (googleapis#3742) by JesseLovelace <[email protected]> - 82fda75 spanner: Expand test coverage for SpannerClient (googleapis#3725) by Nithin Sujir <[email protected]> - 9187488 Fix parent/child spans relationship in Spanner. (googleapis#3690) by Bogdan Drutu <[email protected]> - 3c9bbf2 Revert "Cloud Spanner DML & PartitionedDML support (googleapis#3703... by JesseLovelace <[email protected]> - c163907 Cloud Spanner DML & PartitionedDML support (googleapis#3703) by snehashah16 <[email protected]> - 626e4d1 Regenerate clients (googleapis#3738) by JesseLovelace <[email protected]> - ba388da Remove duplicated folders (googleapis#3732) by Jeff Ching <[email protected]> - d739242 spanner: Options: Fix null dereference, expand test cover... by Nithin Sujir <[email protected]> - 7499c31 Remove Circle CI and Travis configs (googleapis#3723) by Jeff Ching <[email protected]> - 7a72784 Remove dependency on vulnerable version of jackson, upgra... by JesseLovelace <[email protected]> - 2c284dd Add retries to downloadEmulator() to mitigate transient n... by JesseLovelace <[email protected]> - 6955469 bump proto version (googleapis#3718) by Hanzhen Yi <[email protected]> - 9ab7770 Add ArrayUnion/ArrayRemove conformance tests (googleapis#3704) by Sebastian Schmidt <[email protected]> - 36409f5 Bump version to 0.63.1-beta-SNAPSHOT (googleapis#3714) by JesseLovelace <[email protected]> - 94f19b7 Regenerate clients (googleapis#3713) by JesseLovelace <[email protected]> - 7a43f6f Release 1.45.0/0.63.0 (googleapis#3710) by JesseLovelace <[email protected]> - b533c9c Add region tags around an informative sample. (googleapis#3705) by Frank Natividad <[email protected]> - e8cff3f Spanner: Block nested transactions (googleapis#3628) by Nithin Sujir <[email protected]> - 7869364 Migrate Pub/Sub client to ApiFutures. (googleapis#3700) by Anuraag Agrawal <[email protected]> - e310f6d ci: make javadoc not spam us (googleapis#3699) by Michael Darakananda <[email protected]> - fbf8715 Clean up snippets for BatchClient and Spanner (googleapis#3684) by Nithin Sujir <[email protected]> - 3fa1fa0 Bigtable: limit mutation sizes in the client to avoid ove... by Igor Bernstein <[email protected]> - f977c8a bigquery: properly fail when setting TableId's project tw... by Marcello Steiner <[email protected]> - c90b5ba Bigtable: add await replication (googleapis#3658) by Igor Bernstein <[email protected]> - 43d53a8 Bigtable: wrap proto enums (googleapis#3659) by Igor Bernstein <[email protected]> - 6f1a105 Bigtable: add enhanced stub for bigtable table admin clie... by Igor Bernstein <[email protected]> - 68e1717 Kokoro additions (googleapis#3685) by Jeff Ching <[email protected]> - a175980 spanner: expand test coverage for getDatabaseClient() (googleapis#3... by Nithin Sujir <[email protected]> - ae614b3 pubsub: add Publisher.awaitTermination (googleapis#3688) by Michael Darakananda <[email protected]> - 739d519 Add downloadFile sample and reformat storage snippets (googleapis#3... by Frank Natividad <[email protected]> - c901932 storage: include information on a bucket prefix (googleapis#3671) by Frank Natividad <[email protected]> - 713cb88 link to google-cloud-logging from README (googleapis#3681) by Elliotte Rusty Harold <[email protected]> - 5ccfc3a spanner: Add snippets for ReadContext (googleapis#3662) by Nithin Sujir <[email protected]> - ccf68f3 Verify protoc version for batch-generation (googleapis#3676) by Andrea Lin <[email protected]> - 1d7b95f Add Kokoro CI config (googleapis#3664) by Jeff Ching <[email protected]> - e855d48 Bump to next snapshot versions (googleapis#3679) by Garrett Jones <[email protected]> - bbede73 Release 1.44.0/0.62.0 (googleapis#3677) by Garrett Jones <[email protected]> - a420738 Weekly proto refresh (googleapis#3674) by Garrett Jones <[email protected]> - 705acb7 update gax to 1.31/0.48 (googleapis#3675) by Hanzhen Yi <[email protected]> - eadf7bc Adding redis-v1 and video-intelligence-v1p2beta1 (googleapis#3669) by Garrett Jones <[email protected]> - f098439 Regenerate proto/grpc files with protoc 3.6.0 (googleapis#3672) by Andrea Lin <[email protected]> - 4d1da48 Add redis-v1 and video-intelligence-v1p2beta1 to batch (#... by Garrett Jones <[email protected]> - 80c9675 Fixes for ITComputeTest (googleapis#3667) by Andrea Lin <[email protected]> - 509b079 Releasing.md instructions to uncomment nexus-staging-mave... by Andrea Lin <[email protected]> - a2f7111 Add handwritten integration test for Compute GAPIC (googleapis#3660) by Andrea Lin <[email protected]> - 0e58b7d bigtable: RowMutation should allow passing of a Mutation ... by ajaaym <[email protected]> - 37675e9 Bump to snapshot version for development (googleapis#3656) by Andrea Lin <[email protected]> - e4b5266 Release 0.61.0/1.43.0 (googleapis#3653) by Andrea Lin <[email protected]> - 20c329a Javadoc fixes for Bigtable client (googleapis#3652) by Andrea Lin <[email protected]> - d235a82 tell JVM to use less memory when testing (googleapis#3650) by Michael Darakananda <[email protected]> - a9860b3 Bigtable: improve list tables spooler (googleapis#3639) by Igor Bernstein <[email protected]> - 01bec2b Refresh all clients (googleapis#3647) by Andrea Lin <[email protected]> - 0bef731 Bigtable: cosmetic cleanup of table admin (googleapis#3638) by Igor Bernstein <[email protected]> - 3cefb4f Bigtable: table model improvements (googleapis#3640) by Igor Bernstein <[email protected]> - 47bd674 Fix logging integration test failure on when running on G... by Hanzhen Yi <[email protected]> - 124ed55 Revert "[Storage] Bucket lock (googleapis#3574)" (googleapis#3644) by Frank Natividad <[email protected]> - 7f3ab97 Regenerate compute (googleapis#3642) by Andrea Lin <[email protected]> - 9f1a96b [Storage] Bucket lock (googleapis#3574) by Frank Natividad <[email protected]> - 5e3b897 pubsub: clean up after extension gives up (googleapis#3633) by Chris Sainty <[email protected]> - f4bc56d make DatastoreBatchWriter public (googleapis#3387) by Daniel Norberg <[email protected]> - c81ff3e Bigtable: add resource level IAM (googleapis#3624) by Igor Bernstein <[email protected]> - c715fc6 Update signUrl documentation (googleapis#3546) by Jeff Ching <[email protected]> - d7a135a google-cloud-nio: retry on 502 errors, and increase max d... by droazen <[email protected]> - 1f12a83 spanner: Add snippets for Spanner, BatchClient and BatchR... by Nithin Sujir <[email protected]> - 997d2a3 Bigtable: add CRUD for AppProfiles (googleapis#3619) by Igor Bernstein <[email protected]> - e3eedeb Bigtable: add CRUD for clusters (googleapis#3612) by Igor Bernstein <[email protected]> - 8bcc89b Bigtable: clean up consistency token (googleapis#3570) by Igor Bernstein <[email protected]> - 427b155 Bigtable: add CRUD for instances (googleapis#3569) by Igor Bernstein <[email protected]> - b9b4aff fix logging unit tests (googleapis#3630) by Hanzhen Yi <[email protected]> - ff2ed5e bump to snapshot version (googleapis#3625) by Hanzhen Yi <[email protected]> - 4cd518d Release 0.60.0 (googleapis#3623) by Hanzhen Yi <[email protected]> - 3d1e971 batch generation before release (googleapis#3622) by Hanzhen Yi <[email protected]> - 08a5e46 [Storage] Make StorageIT easier to setup with new project... by Frank Natividad <[email protected]> - 4eca7a4 upgrade auth version (googleapis#3606) by Elliotte Rusty Harold <[email protected]> - 8f44a9a removing word "natural" from product name (googleapis#3610) by Vincent <[email protected]> - 4c3e3d1 Fix documentation for setParallelPullCount (googleapis#3542) by Jonas Wiklund <[email protected]> - 5fc0dbd bump version for development (googleapis#3599) by Michael Darakananda <[email protected]> (And 1760 more changes) GitOrigin-RevId: b2056bd147466fec8a817885fc5da0f5f39b5730

Spanner: Block nested transactions

89f1dd6

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]>

nithinsujir requested a review from pongad as a code owner August 31, 2018 16:08

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 31, 2018

nithinsujir requested a review from snehashah16 August 31, 2018 16:39

snehashah16 reviewed Aug 31, 2018

View reviewed changes

jcorwin reviewed Sep 13, 2018

View reviewed changes

nsujir added 2 commits September 14, 2018 09:03

Add a flag to override default behaviour of nested transaction blocking

1062734

Fix codacy warnings

604e062

snehashah16 reviewed Sep 14, 2018

View reviewed changes

nsujir and others added 2 commits September 14, 2018 11:26

Add detailed comment for allowNestedTransaction(), make test names cl…

72409fd

…earer

Merge branch 'master' into block-nested-2

9324edb

snehashah16 approved these changes Sep 17, 2018

View reviewed changes

pongad reviewed Sep 17, 2018

View reviewed changes

Remove hasPendingTransaction unconditionally

d2dd3dc

Merge branch 'master' into block-nested-2

f0b159c

pongad merged commit e8cff3f into googleapis:master Sep 19, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Spanner: Block nested transactions #3628

Spanner: Block nested transactions #3628

nithinsujir commented Aug 31, 2018 •

edited

Loading

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

snehashah16 left a comment

This comment was marked as spam.

This comment was marked as spam.

nithinsujir commented Sep 18, 2018

pongad commented Sep 18, 2018

nithinsujir commented Sep 19, 2018

Spanner: Block nested transactions #3628

Spanner: Block nested transactions #3628

Conversation

nithinsujir commented Aug 31, 2018 • edited Loading

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

snehashah16 left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

nithinsujir commented Sep 18, 2018

pongad commented Sep 18, 2018

nithinsujir commented Sep 19, 2018

nithinsujir commented Aug 31, 2018 •

edited

Loading