feat(spanner): define IsolationLevel enum for Spanner transactions #15853

prathmeshcc · 2026-01-05T19:08:12Z

This PR introduces the IsolationLevel enum class in google/cloud/spanner/isolation_level.h.

This enum maps to the google.spanner.v1.TransactionOptions.IsolationLevel protobuf and defines the supported isolation levels for Cloud Spanner read-write transactions:

kUnspecified
kSerializable
kRepeatableRead

This addition is a prerequisite for allowing users to configure the isolation level (specifically Repeatable Read) in transaction options.

gemini-code-assist · 2026-01-05T19:08:37Z

Summary of Changes

Hello @prathmeshcc, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the Google Cloud Spanner C++ client library by introducing explicit support for transaction isolation levels. It defines a new IsolationLevel enum and integrates it into the transaction options, allowing users to configure the desired isolation behavior, such as Repeatable Read, for their read-write transactions. This change lays the groundwork for more granular control over transaction semantics.

Highlights

New IsolationLevel Enum: Introduced google::cloud::spanner::IsolationLevel enum class, mirroring google.spanner.v1.TransactionOptions.IsolationLevel with kUnspecified, kSerializable, and kRepeatableRead.
Configurable Transaction Isolation: Added the ability to specify the isolation level for read-write transactions through Transaction::ReadWriteOptions and TransactionIsolationLevelOption.
Integration and Unit Tests: Included new integration tests in client_integration_test.cc and unit tests in transaction_test.cc to validate the correct application and precedence of isolation levels.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces an IsolationLevel enum and the corresponding TransactionIsolationLevelOption to allow users to configure the isolation level for read-write transactions. The changes are well-structured and follow existing patterns in the codebase. The new functionality is well-tested with new unit and integration tests that verify the option handling and precedence. The implementation appears correct and complete. I have one minor suggestion to remove some debugging output from the unit tests.

google/cloud/spanner/transaction_test.cc

codecov · 2026-01-05T20:10:18Z

Codecov Report

❌ Patch coverage is 97.08738% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.95%. Comparing base (752f0ed) to head (7451e09).

Files with missing lines	Patch %	Lines
google/cloud/spanner/transaction.cc	90.32%	3 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##             main   #15853    +/-   ##
========================================
  Coverage   92.95%   92.95%            
========================================
  Files        2458     2458            
  Lines      227974   228074   +100     
========================================
+ Hits       211903   212003   +100     
  Misses      16071    16071

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

devbww · 2026-01-05T19:55:47Z

google/cloud/spanner/transaction.cc

+    opts.set_isolation_level(
+        static_cast<google::spanner::v1::TransactionOptions::IsolationLevel>(
+            isolation_level));


We strive to not tie the C++ enum values to the protobuf enum values. See, for example, LockHint, OrderBy, ReadLockMode, ReplicaType, RequestPriority, etc. Please continue that by introducing an explicit mapping here (removing the static_cast and specific enum class values).

google/cloud/spanner/integration_tests/client_integration_test.cc

devbww · 2026-01-05T20:23:14Z

google/cloud/spanner/integration_tests/client_integration_test.cc

+        // std::cout << "Transaction is active." << std::endl;
+        return Mutations{};
+      },
+      Options{}.set<TransactionIsolationLevelOption>(


Would this test behave any differently if this was ignored? That is, what are we testing?

You're right that this test would pass with the default isolation level. Its purpose is to act as a smoke test to ensure the TransactionIsolationLevelOption is correctly serialized by the client and accepted by the backend without errors. I've added a comment to make this intent clear.

devbww · 2026-01-05T20:23:38Z

google/cloud/spanner/integration_tests/client_integration_test.cc

+        }
+        return Mutations{};
+      },
+      Options{}.set<TransactionIsolationLevelOption>(


devbww · 2026-01-05T20:33:35Z

google/cloud/spanner/transaction.cc

+  *selector.mutable_begin() =
+      MakeOpts(std::move(opts.rw_opts_), opts.isolation_level_);


[GitHub seems to have dropped a comment here. Apologies if you see this twice.]

It seems weird that the isolation level appears to only be useful in read-write mode yet it wasn't made part of message ReadWrite. Is that appearance true (and the protos should probably be fixed, in which case we would not need any special treatment here), or are we being too specific in making the isolation mode a member of ReadWriteOptions?

You're correct, Transaction isolation levels are only meaningful for read-write transactions.

The original implementation was a bit too general. It allowed IsolationLevel to be passed in contexts where it didn't apply, which could be confusing.

My latest changes address this directly. I have moved isolation_level_ to be a private member of Transaction::ReadWriteOptions. This tightens up the API and makes the class structure more accurately reflect the constraints of the underlying Spanner API. The Transaction constructor now correctly pulls the isolation level from the ReadWriteOptions struct, ensuring it's only ever configured in the correct context.

OK. If there is no chance to correct the proto definition, then we should certainly make sure that a comment is added to clarify that isolation_level only applies to the ReadWrite mode.

This commit improves the API design by integrating `IsolationLevel` directly into `Transaction::ReadWriteOptions`. Previously, `IsolationLevel` was passed as a separate argument to some `Transaction` constructors, even though it is semantically relevant only to read-write transactions. This could lead to ambiguity or misuse. By making `IsolationLevel` a member of `ReadWriteOptions`, the API more accurately reflects the underlying Spanner transaction semantics, enforcing that isolation levels are configured solely within the context of read-write transactions. This enhances type safety and improves the clarity of the client library's API. This change addresses reviewer feedback regarding the logical grouping of `IsolationLevel` within the `ReadWriteOptions` message.#

scotthart · 2026-01-12T22:11:02Z

google/cloud/spanner/transaction.h


    explicit ReadWriteOptions(ReadLockMode read_lock_mode);

+    explicit ReadWriteOptions(IsolationLevel isolation_level)


Let's remove this constructor and rely on the WithIsolationLevel mutator to apply the desired setting. That way we avoid a combinatorial growth of constructors with different options.

Ditto. It looks like it is only used once (from a test).

scotthart · 2026-01-12T22:12:39Z

google/cloud/spanner/transaction.h

 */
 class Transaction {
 public:
+  enum class IsolationLevel {


We should add some documentation for this enum.

scotthart · 2026-01-12T22:17:54Z

google/cloud/spanner/transaction.h

  /// @copydoc Transaction(ReadOnlyOptions)
  explicit Transaction(ReadWriteOptions opts);
  /// @copydoc Transaction(ReadOnlyOptions)
+  explicit Transaction(ReadWriteOptions opts, IsolationLevel isolation_level);


This constructor is unnecessary as ReadWriteOptions already contains the IsolationLevel.

scotthart · 2026-01-12T22:18:55Z

google/cloud/spanner/transaction.h

    // A tag used for collecting statistics about the transaction.
    ReadWriteOptions& WithTag(absl::optional<std::string> tag);

+    ReadWriteOptions& WithIsolationLevel(IsolationLevel isolation_level);


Add a brief comment for this function to flesh out the generated documentation.

scotthart · 2026-01-12T22:20:26Z

google/cloud/spanner/options.h

  using Type = bool;
 };

+/**


We don't need to define this Option as ReadWriteOptions::WithIsolationLevel allows the user to set the value where appropriate.

That depends on whether the isolation level is something you might want to set for a client (and hence avoid adding WithIsolationLevel() calls everywhere), or whether it is something that should only be considered on a transaction-by-transaction basis.

That's a fair point, and I'm not sure at present which is preferable. As is the nature of public APIs, we can always add IsolationLevel as an Option later if we get feedback that calling WithIsolationLevel all the time is a point of friction.

I checked the implementation for other languages and they have introduced a client level option : https://docs.cloud.google.com/spanner/docs/use-repeatable-read-isolation

Seeing as our Spanner library has a convention for having this type of Option, let's go ahead and add/keep it to maintain a consistent look and feel for the user.

scotthart · 2026-01-12T22:20:33Z

google/cloud/spanner/options.h

 #include "google/cloud/spanner/polling_policy.h"
 #include "google/cloud/spanner/request_priority.h"
 #include "google/cloud/spanner/retry_policy.h"
+#include "google/cloud/spanner/transaction.h"


Remove this include as well.

scotthart · 2026-01-12T22:26:05Z

google/cloud/spanner/transaction.cc

+      *isolation_level != Transaction::IsolationLevel::kUnspecified) {
+    opts.set_isolation_level(ProtoIsolationLevel(isolation_level));
+  } else if (current.has<TransactionIsolationLevelOption>()) {
+    opts.set_isolation_level(


Remove this else if block since we're removing TransactionIsolationLevelOption.

scotthart · 2026-01-12T22:29:41Z

google/cloud/spanner/transaction.cc

+}
+
+Transaction::Transaction(ReadWriteOptions opts,
+                         IsolationLevel isolation_level) {


IsolationLevel is present as a member field of ReadWriteOptions, we shouldn't need to duplicate it in the parameters in a new constructor.

devbww

This PR introduces the IsolationLevel enum class in google/cloud/spanner/isolation_level.h.

Update the PR description to reflect the state of the PR.

devbww · 2026-01-12T23:29:49Z

google/cloud/spanner/integration_tests/client_integration_test.cc

+// A smoke test to verify that the TransactionIsolationLevelOption is
+// correctly processed and accepted by the Spanner backend. It does not


Would this test behave any differently if this was ignored? That is, what are we testing?

You're right that this test would pass with the default isolation level. Its purpose is to act as a smoke test to ensure the TransactionIsolationLevelOption is correctly serialized by the client and accepted by the backend without errors.

This does not ensure the option is correctly serialized by the client. That's what the Transaction unit tests are for.

It also doesn't test that the backend does anything in particular. That is for a backend test.

If there was anything the client could do to distinguish the behavior of different isolation levels then perhaps we could do something here, but even that would seem to be out of scope for client tests unless there was some client-side code involved.

So, let's remove it.

devbww · 2026-01-12T23:30:05Z

google/cloud/spanner/integration_tests/client_integration_test.cc

+// A smoke test to verify that the TransactionIsolationLevelOption is
+// correctly processed and accepted by the Spanner backend. It does not


devbww · 2026-01-12T23:34:31Z

google/cloud/spanner/options.h

  using Type = bool;
 };

+/**


That depends on whether the isolation level is something you might want to set for a client (and hence avoid adding WithIsolationLevel() calls everywhere), or whether it is something that should only be considered on a transaction-by-transaction basis.

devbww · 2026-01-12T23:36:39Z

google/cloud/spanner/transaction.cc

    opts.set_exclude_txn_from_change_streams(true);
  }
+  if (isolation_level &&
+      *isolation_level != Transaction::IsolationLevel::kUnspecified) {


Why make this extra guard when we don't do the same thing when current is unspecified?

devbww · 2026-01-12T23:40:23Z

google/cloud/spanner/transaction.cc

+  *selector.mutable_begin() =
+      MakeOpts(std::move(opts.rw_opts_), opts.isolation_level_);


OK. If there is no chance to correct the proto definition, then we should certainly make sure that a comment is added to clarify that isolation_level only applies to the ReadWrite mode.

devbww · 2026-01-12T23:42:55Z

google/cloud/spanner/transaction.cc

+  auto const route_to_leader = true;  // read-write
+  impl_ = std::make_shared<spanner_internal::TransactionImpl>(
+      std::move(selector), route_to_leader,
+      std::move(opts.tag_).value_or(std::string()));


Don't duplicate all of this ... just forward to Transaction(opts, opts.isolation_level_), if it is needed at all.

devbww · 2026-01-12T23:44:47Z

google/cloud/spanner/transaction.h


    explicit ReadWriteOptions(ReadLockMode read_lock_mode);

+    explicit ReadWriteOptions(IsolationLevel isolation_level)


Ditto. It looks like it is only used once (from a test).

devbww · 2026-01-12T23:45:53Z

google/cloud/spanner/transaction_test.cc

      google::spanner::v1::TransactionOptions_ReadWrite::OPTIMISTIC);
 }

+TEST(Transaction, ReadWriteOptionsWithIsolationLevel) {


How is this any different from Transaction.IsolationLevel?

devbww · 2026-01-12T23:46:55Z

google/cloud/spanner/transaction_test.cc

      });
 }

+TEST(Transaction, IsolationLevel) {


How is this any different from Transaction.IsolationLevelPrecedence case 1?

product-auto-label bot added the api: spanner Issues related to the Spanner API. label Jan 5, 2026

prathmeshcc temporarily deployed to false January 5, 2026 19:08 — with GitHub Actions Inactive

gemini-code-assist bot reviewed Jan 5, 2026

View reviewed changes

google/cloud/spanner/transaction_test.cc Show resolved Hide resolved

devbww reviewed Jan 5, 2026

View reviewed changes

prathmeshcc force-pushed the feat/spanner-isolation-level branch from ceb7ee6 to b89fdd9 Compare January 12, 2026 17:38

prathmeshcc marked this pull request as ready for review January 12, 2026 17:48

prathmeshcc requested a review from a team as a code owner January 12, 2026 17:48

prathmeshcc added 2 commits January 12, 2026 19:10

feat(spanner): add support for snapshot isolation

e7407c6

prathmeshcc force-pushed the feat/spanner-isolation-level branch from b89fdd9 to 7451e09 Compare January 12, 2026 19:16

scotthart requested changes Jan 12, 2026

View reviewed changes

devbww reviewed Jan 12, 2026

View reviewed changes

		*selector.mutable_begin() =
		MakeOpts(std::move(opts.rw_opts_), opts.isolation_level_);


		explicit ReadWriteOptions(ReadLockMode read_lock_mode);

		explicit ReadWriteOptions(IsolationLevel isolation_level)

		// A smoke test to verify that the TransactionIsolationLevelOption is
		// correctly processed and accepted by the Spanner backend. It does not

feat(spanner): define IsolationLevel enum for Spanner transactions #15853

Are you sure you want to change the base?

feat(spanner): define IsolationLevel enum for Spanner transactions #15853

Conversation

prathmeshcc commented Jan 5, 2026

Uh oh!

gemini-code-assist bot commented Jan 5, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

codecov bot commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devbww left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jan 5, 2026 •

edited

Loading