feat: support isolation level REPEATABLE_READ for R/W transactions #3670

shobhitsg · 2025-03-03T03:18:24Z

Isolation level support added in various write APIs:

write
writeWithOptions
writeAtleastOnceWithOptions
readWriteTransaction
transactionManager
transactionManagerAsync
runAsync

Note: Samples and integration tests will be added in separate PRs.

olavloite

Please change the title of the pull request to feat: support isolation level REPEATABLE_READ for R/W transactions

The feat(spanner): prefix should only be used in mono-repos that contain clients for multiple different products, as it is needed to indicate for which product the feature is. This repository only contains a Spanner client, so the product designation is not needed.

olavloite · 2025-03-03T19:01:46Z

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

+  public static TransactionOption repeatableReadIsolationLevel() {
+    return REPEATABLE_READ_OPTION;
+  }
+
+  /**
+   * Specifying this instructs the transaction to request Serializable Isolation Level from the
+   * backend.
+   */
+  public static TransactionOption serializableIsolationLevel() {
+    return SERIALIZABLE_OPTION;
+  }


Instead of creating two methods, one for each isolation level, this should be one method that takes an IsolationLevel input argument. This means that any future isolation levels that we add can be used without any changes.

I completely agree with your point. However, IMO, including request options that are not yet fully supported(when we add more IsolationLevel) by the library, even for short duration(e.g. development if that's even required?), can create confusion among lib users. Though the backend will reject unsupported options, their presence without clear documentation could mislead developers who often rely on request options(as it's a valid contract) to determine functionality. Please let me know if I've misunderstood anything and I'd like to know your POV. Here, I've assumed that the proto will always be merged in the main branch before any development could start.

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

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

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

olavloite · 2025-03-03T19:36:02Z

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

+          if (!isValidIsolationLevelOption.test(transactionOption)) {
+            throw SpannerExceptionFactory.newSpannerException(
+                ErrorCode.INVALID_ARGUMENT, INVALID_ISOLATION_LEVEL_MESSAGE);
+          }


This should really not be necessary. IsolationLevel is an enum, so it should be impossible to set any other value (and if anyone sets one of the UNRECOGNIZED or UNSPECIFIED values, then just let the error that is returned by Spanner propagate to the application)

This is required with the existing implementation as IsolationLevel enum is not the part of the request currently. Thus, I have to validate the correctness of the com.google.cloud.spanner.TransactionOption request. It's not about what isolation level is passed, it was more about that TransactionOption shouldn't of any other type except of IsolationLevel type.
Anyways, this will be removed in the subsequent commit since I'll be making IsolationLevel change in the request as you suggested in the previous comment.

...ner/src/test/java/com/google/cloud/spanner/DatabaseClientImplWithTransactionOptionsTest.java

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

shobhitsg · 2025-03-10T02:48:58Z

@olavloite @sakthivelmanii I have implemented the suggested revisions. PTAL.

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

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

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

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

olavloite · 2025-03-11T09:46:26Z

...ner/src/test/java/com/google/cloud/spanner/DatabaseClientImplWithTransactionOptionsTest.java

+  public void setUp() {
+    mockSpanner.reset();
+    mockSpanner.removeAllExecutionTimes();
+    SpannerOptions.Builder spannerOptionsBuilder =


nit: I think that you can move the creation of the Spanner instances to a @BeforeClass method (and the same then also with the spanner.close()

I'll raise a separate PR for this minor refactoring.

...ner/src/test/java/com/google/cloud/spanner/DatabaseClientImplWithTransactionOptionsTest.java

olavloite

LGTM, but please change the name of the Options#isolationLevelOption(..) method to Options#isolationLevel(..) before merging.

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

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

…tions.java Co-authored-by: Knut Olav Løite <koloite@gmail.com>

…annerOptions.java Co-authored-by: Knut Olav Løite <koloite@gmail.com>

…ns.isolationLevel

olavloite · 2025-03-17T14:11:06Z

Last build errors are unrelated, created #3690

| Package | Type | Package file | Manager | Update | Change | |---|---|---|---|---|---| | [com.google.cloud:google-cloud-spanner](https://github.com/googleapis/java-spanner) | dependencies | misk/gradle/libs.versions.toml | gradle | minor | `6.88.0` -> `6.89.0` | | [com.google.cloud:google-cloud-datastore](https://github.com/googleapis/java-datastore) | dependencies | misk/gradle/libs.versions.toml | gradle | patch | `2.27.0` -> `2.27.1` | | [software.amazon.awssdk:sdk-core](https://aws.amazon.com/sdkforjava) | dependencies | misk/gradle/libs.versions.toml | gradle | patch | `2.31.4` -> `2.31.6` | | [software.amazon.awssdk:sqs](https://aws.amazon.com/sdkforjava) | dependencies | misk/gradle/libs.versions.toml | gradle | patch | `2.31.4` -> `2.31.6` | | [software.amazon.awssdk:dynamodb-enhanced](https://aws.amazon.com/sdkforjava) | dependencies | misk/gradle/libs.versions.toml | gradle | patch | `2.31.4` -> `2.31.6` | | [software.amazon.awssdk:dynamodb](https://aws.amazon.com/sdkforjava) | dependencies | misk/gradle/libs.versions.toml | gradle | patch | `2.31.4` -> `2.31.6` | | [software.amazon.awssdk:aws-core](https://aws.amazon.com/sdkforjava) | dependencies | misk/gradle/libs.versions.toml | gradle | patch | `2.31.4` -> `2.31.6` | | [software.amazon.awssdk:bom](https://aws.amazon.com/sdkforjava) | dependencies | misk/gradle/libs.versions.toml | gradle | patch | `2.31.4` -> `2.31.6` | | [software.amazon.awssdk:auth](https://aws.amazon.com/sdkforjava) | dependencies | misk/gradle/libs.versions.toml | gradle | patch | `2.31.4` -> `2.31.6` | --- ### Release Notes <details> <summary>googleapis/java-spanner (com.google.cloud:google-cloud-spanner)</summary> ### [`v6.89.0`](https://github.com/googleapis/java-spanner/blob/HEAD/CHANGELOG.md#6890-2025-03-20) ##### Features - Enable ALTS hard bound token in DirectPath ([#3645](googleapis/java-spanner#3645)) ([42cc961](googleapis/java-spanner@42cc961)) - Next release from main branch is 6.89.0 ([#3669](googleapis/java-spanner#3669)) ([7a8a29b](googleapis/java-spanner@7a8a29b)) - Support isolation level REPEATABLE_READ for R/W transactions ([#3670](googleapis/java-spanner#3670)) ([e62f5ab](googleapis/java-spanner@e62f5ab)) ##### Bug Fixes - **deps:** Update the Java code generator (gapic-generator-java) to 2.55.1 ([b959f4c](googleapis/java-spanner@b959f4c)) - Revert the ALTS bound token enablement ([#3679](googleapis/java-spanner#3679)) ([183c1f0](googleapis/java-spanner@183c1f0)) ##### Performance Improvements - Get database dialect using multiplexed session ([#3684](googleapis/java-spanner#3684)) ([f641a40](googleapis/java-spanner@f641a40)) - Skip gRPC trailers for StreamingRead & ExecuteStreamingSql ([#3661](googleapis/java-spanner#3661)) ([bd4b1f5](googleapis/java-spanner@bd4b1f5)) ##### Dependencies - Update dependency com.google.cloud:sdk-platform-java-config to v3.45.1 ([#3689](googleapis/java-spanner#3689)) ([67188df](googleapis/java-spanner@67188df)) </details> <details> <summary>googleapis/java-datastore (com.google.cloud:google-cloud-datastore)</summary> ### [`v2.27.1`](https://github.com/googleapis/java-datastore/blob/HEAD/CHANGELOG.md#2271-2025-03-18) ##### Bug Fixes - **deps:** Update the Java code generator (gapic-generator-java) to 2.55.1 ([ba1ad98](googleapis/java-datastore@ba1ad98)) ##### Dependencies - Update dependency com.google.cloud:sdk-platform-java-config to v3.45.1 ([#1791](googleapis/java-datastore#1791)) ([ab5ac8e](googleapis/java-datastore@ab5ac8e)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - "after 6pm every weekday,before 2am every weekday" in timezone Australia/Melbourne, Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Never, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). GitOrigin-RevId: 5b5b3e1fd7177ff56a483be663b20b28d243550c

shobhitsg requested review from a team as code owners March 3, 2025 03:18

product-auto-label bot added size: l api: spanner labels Mar 3, 2025

olavloite reviewed Mar 3, 2025

View reviewed changes

sakthivelmanii reviewed Mar 4, 2025

View reviewed changes

...ner/src/test/java/com/google/cloud/spanner/DatabaseClientImplWithTransactionOptionsTest.java Outdated Show resolved Hide resolved

google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java Outdated Show resolved Hide resolved

shobhitsg changed the title ~~feat(spanner): Support REPEATABLE_READ for R/W transaction~~ feat: Support REPEATABLE_READ for R/W transaction Mar 4, 2025

shobhitsg changed the title ~~feat: Support REPEATABLE_READ for R/W transaction~~ feat: support isolation level REPEATABLE_READ for R/W transactions Mar 4, 2025

product-auto-label bot added size: xl and removed size: l labels Mar 9, 2025

shobhitsg force-pushed the repeatable_read_isolation branch 3 times, most recently from 818d2fc to 6879a0a Compare March 9, 2025 12:27

olavloite reviewed Mar 11, 2025

View reviewed changes

shobhitsg force-pushed the repeatable_read_isolation branch 4 times, most recently from 02b88a1 to 17eed16 Compare March 11, 2025 13:53

olavloite approved these changes Mar 11, 2025

View reviewed changes

google-cloud-spanner/src/main/java/com/google/cloud/spanner/Options.java Outdated Show resolved Hide resolved

google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java Outdated Show resolved Hide resolved

sakthivelmanii approved these changes Mar 17, 2025

View reviewed changes

shobhitsg and others added 8 commits March 17, 2025 16:18

feat(spanner): Support REPEATABLE_READ for RW transaction

23494f5

feat: addressed review comments

cd8ee25

feat: addressed review comments and added missing javadoc

97bcd9f

Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Op…

2df96f9

…tions.java Co-authored-by: Knut Olav Løite <koloite@gmail.com>

Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Sp…

d63e36e

…annerOptions.java Co-authored-by: Knut Olav Løite <koloite@gmail.com>

feat: code reformat and minor test cases fix due to renaming of Optio…

afdf0eb

…ns.isolationLevel

feat: Options method renamed in javadoc

12f8a93

code formatting fixed

Loading
Loading status checks…

515db9c

shobhitsg force-pushed the repeatable_read_isolation branch from 5e94c1d to 515db9c Compare March 17, 2025 10:48

olavloite added the kokoro:force-run label Mar 17, 2025

yoshi-kokoro removed the kokoro:force-run label Mar 17, 2025

olavloite added the kokoro:force-run label Mar 17, 2025

yoshi-kokoro removed the kokoro:force-run label Mar 17, 2025

olavloite merged commit e62f5ab into googleapis:main Mar 17, 2025
31 of 33 checks passed

release-please bot mentioned this pull request Mar 17, 2025

chore(main): release 6.89.0 #3685

Merged

suztomo mentioned this pull request Mar 20, 2025

protobuf ci 4.29.x #3703

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support isolation level REPEATABLE_READ for R/W transactions #3670

feat: support isolation level REPEATABLE_READ for R/W transactions #3670

shobhitsg commented Mar 3, 2025

olavloite left a comment

olavloite Mar 3, 2025

shobhitsg Mar 7, 2025 •

edited

Loading

olavloite Mar 3, 2025

shobhitsg Mar 7, 2025

shobhitsg commented Mar 10, 2025

olavloite Mar 11, 2025

shobhitsg Mar 11, 2025

olavloite left a comment

olavloite commented Mar 17, 2025

feat: support isolation level REPEATABLE_READ for R/W transactions #3670

feat: support isolation level REPEATABLE_READ for R/W transactions #3670

Conversation

shobhitsg commented Mar 3, 2025

olavloite left a comment

Choose a reason for hiding this comment

olavloite Mar 3, 2025

Choose a reason for hiding this comment

shobhitsg Mar 7, 2025 • edited Loading

Choose a reason for hiding this comment

olavloite Mar 3, 2025

Choose a reason for hiding this comment

shobhitsg Mar 7, 2025

Choose a reason for hiding this comment

shobhitsg commented Mar 10, 2025

olavloite Mar 11, 2025

Choose a reason for hiding this comment

shobhitsg Mar 11, 2025

Choose a reason for hiding this comment

olavloite left a comment

Choose a reason for hiding this comment

olavloite commented Mar 17, 2025

shobhitsg Mar 7, 2025 •

edited

Loading