From e7407c6f496cb1d16d48bce02db89569f5abc31b Mon Sep 17 00:00:00 2001 From: Prathmesh Chandwade Date: Mon, 5 Jan 2026 18:48:28 +0000 Subject: [PATCH 1/3] feat(spanner): add support for snapshot isolation --- google/cloud/spanner/CMakeLists.txt | 1 + .../spanner/google_cloud_cpp_spanner.bzl | 1 + .../client_integration_test.cc | 36 ++++++++++++ google/cloud/spanner/isolation_level.h | 43 +++++++++++++++ google/cloud/spanner/options.h | 11 ++++ google/cloud/spanner/transaction.cc | 20 +++++-- google/cloud/spanner/transaction.h | 7 +++ google/cloud/spanner/transaction_test.cc | 55 +++++++++++++++++++ 8 files changed, 170 insertions(+), 4 deletions(-) create mode 100644 google/cloud/spanner/isolation_level.h diff --git a/google/cloud/spanner/CMakeLists.txt b/google/cloud/spanner/CMakeLists.txt index acb029537fecb..8039d1c470809 100644 --- a/google/cloud/spanner/CMakeLists.txt +++ b/google/cloud/spanner/CMakeLists.txt @@ -173,6 +173,7 @@ add_library( internal/tuple_utils.h interval.cc interval.h + isolation_level.h json.h keys.cc keys.h diff --git a/google/cloud/spanner/google_cloud_cpp_spanner.bzl b/google/cloud/spanner/google_cloud_cpp_spanner.bzl index 2225628fa9d8e..a2208d8e33ae4 100644 --- a/google/cloud/spanner/google_cloud_cpp_spanner.bzl +++ b/google/cloud/spanner/google_cloud_cpp_spanner.bzl @@ -94,6 +94,7 @@ google_cloud_cpp_spanner_hdrs = [ "internal/transaction_impl.h", "internal/tuple_utils.h", "interval.h", + "isolation_level.h", "json.h", "keys.h", "lock_hint.h", diff --git a/google/cloud/spanner/integration_tests/client_integration_test.cc b/google/cloud/spanner/integration_tests/client_integration_test.cc index 2c114ca56f18b..e97139a2980d1 100644 --- a/google/cloud/spanner/integration_tests/client_integration_test.cc +++ b/google/cloud/spanner/integration_tests/client_integration_test.cc @@ -16,6 +16,7 @@ #include "google/cloud/spanner/client.h" #include "google/cloud/spanner/database.h" #include "google/cloud/spanner/mutations.h" +#include "google/cloud/spanner/isolation_level.h" #include "google/cloud/spanner/options.h" #include "google/cloud/spanner/testing/database_integration_test.h" #include "google/cloud/credentials.h" @@ -240,6 +241,25 @@ TEST_F(ClientIntegrationTest, MultipleInserts) { RowType(4, "test-fname-4", "test-lname-4"))); } +/// @test Verify that TransactionIsolationLevel works as expected. +TEST_F(ClientIntegrationTest, TransactionIsolationLevel) { + auto& client = *client_; + auto commit = client.Commit( + [&](Transaction const& txn) -> StatusOr { + // Perform a read to ensure the transaction is active on the server. + auto rows = client.ExecuteQuery(txn, SqlStatement("SELECT 1")); + for (auto const& row : rows) { + if (!row) return row.status(); + } + // std::cout << "Transaction is active." << std::endl; + return Mutations{}; + }, + Options{}.set( + IsolationLevel::kRepeatableRead)); + EXPECT_THAT(commit, StatusIs(StatusCode::kOk)); +} + + /// @test Verify that Client::Rollback works as expected. TEST_F(ClientIntegrationTest, TransactionRollback) { ASSERT_NO_FATAL_FAILURE(InsertTwoSingers()); @@ -1399,6 +1419,22 @@ TEST_F(PgClientIntegrationTest, FineGrainedAccessControl) { ASSERT_STATUS_OK(metadata); } +/// @test Verify that TransactionIsolationLevel works as expected. +TEST_F(PgClientIntegrationTest, TransactionIsolationLevel) { + auto& client = *client_; + auto commit = client.Commit( + [&](Transaction const& txn) -> StatusOr { + auto rows = client.ExecuteQuery(txn, SqlStatement("SELECT 1")); + for (auto const& row : rows) { + if (!row) return row.status(); + } + return Mutations{}; + }, + Options{}.set( + IsolationLevel::kRepeatableRead)); + EXPECT_THAT(commit, StatusIs(StatusCode::kOk)); +} + /// @test Verify "FOREIGN KEY" "ON DELETE CASCADE". TEST_F(ClientIntegrationTest, ForeignKeyDeleteCascade) { spanner_admin::DatabaseAdminClient admin_client( diff --git a/google/cloud/spanner/isolation_level.h b/google/cloud/spanner/isolation_level.h new file mode 100644 index 0000000000000..3648eac54709b --- /dev/null +++ b/google/cloud/spanner/isolation_level.h @@ -0,0 +1,43 @@ +// google/cloud/spanner/isolation_level.h +// +// Copyright 2025 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_SPANNER_ISOLATION_LEVEL_H +#define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_SPANNER_ISOLATION_LEVEL_H + +#include "google/cloud/spanner/version.h" + +namespace google { +namespace cloud { +namespace spanner { +GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN + +/** + * The isolation level for read-write transactions. + * + * @see https://cloud.google.com/spanner/docs/reference/rpc/google.spanner.v1#google.spanner.v1.TransactionOptions.IsolationLevel + */ +enum class IsolationLevel { + kUnspecified = 0, + kSerializable = 1, + kRepeatableRead = 2, +}; + +GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END +} // namespace spanner +} // namespace cloud +} // namespace google + +#endif // GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_SPANNER_ISOLATION_LEVEL_H diff --git a/google/cloud/spanner/options.h b/google/cloud/spanner/options.h index 8111acfc5df15..474f200f42b14 100644 --- a/google/cloud/spanner/options.h +++ b/google/cloud/spanner/options.h @@ -41,6 +41,7 @@ #include "google/cloud/spanner/backoff_policy.h" #include "google/cloud/spanner/directed_read_replicas.h" #include "google/cloud/spanner/internal/session.h" +#include "google/cloud/spanner/isolation_level.h" #include "google/cloud/spanner/lock_hint.h" #include "google/cloud/spanner/order_by.h" #include "google/cloud/spanner/polling_policy.h" @@ -425,6 +426,16 @@ struct CommitReturnStatsOption { using Type = bool; }; +/** + * Option for `google::cloud::Options` to set the isolation level for + * read-write transactions. + * + * @ingroup google-cloud-spanner-options + */ +struct TransactionIsolationLevelOption { + using Type = IsolationLevel; +}; + /** * Option for `google::cloud::Options` to set the amount of latency this request * is willing to incur in order to improve throughput. If this field is not set, diff --git a/google/cloud/spanner/transaction.cc b/google/cloud/spanner/transaction.cc index 07d9c9edd464d..aedffcf9dcbc2 100644 --- a/google/cloud/spanner/transaction.cc +++ b/google/cloud/spanner/transaction.cc @@ -62,13 +62,22 @@ google::spanner::v1::TransactionOptions MakeOpts( } google::spanner::v1::TransactionOptions MakeOpts( - google::spanner::v1::TransactionOptions_ReadWrite rw_opts) { + google::spanner::v1::TransactionOptions_ReadWrite rw_opts, + IsolationLevel isolation_level) { google::spanner::v1::TransactionOptions opts; *opts.mutable_read_write() = std::move(rw_opts); auto const& current = internal::CurrentOptions(); if (current.get()) { opts.set_exclude_txn_from_change_streams(true); } + if (isolation_level == IsolationLevel::kUnspecified) { + isolation_level = current.get(); + } + if (isolation_level != IsolationLevel::kUnspecified) { + opts.set_isolation_level( + static_cast( + isolation_level)); + } return opts; } @@ -129,7 +138,8 @@ Transaction::Transaction(ReadOnlyOptions opts) { Transaction::Transaction(ReadWriteOptions opts) { google::spanner::v1::TransactionSelector selector; - *selector.mutable_begin() = MakeOpts(std::move(opts.rw_opts_)); + *selector.mutable_begin() = + MakeOpts(std::move(opts.rw_opts_), opts.isolation_level_); auto const route_to_leader = true; // read-write impl_ = std::make_shared( std::move(selector), route_to_leader, @@ -138,7 +148,8 @@ Transaction::Transaction(ReadWriteOptions opts) { Transaction::Transaction(Transaction const& txn, ReadWriteOptions opts) { google::spanner::v1::TransactionSelector selector; - *selector.mutable_begin() = MakeOpts(std::move(opts.rw_opts_)); + *selector.mutable_begin() = + MakeOpts(std::move(opts.rw_opts_), opts.isolation_level_); auto const route_to_leader = true; // read-write impl_ = std::make_shared( *txn.impl_, std::move(selector), route_to_leader, @@ -155,7 +166,8 @@ Transaction::Transaction(SingleUseOptions opts) { Transaction::Transaction(ReadWriteOptions opts, SingleUseCommitTag) { google::spanner::v1::TransactionSelector selector; - *selector.mutable_single_use() = MakeOpts(std::move(opts.rw_opts_)); + *selector.mutable_single_use() = + MakeOpts(std::move(opts.rw_opts_), opts.isolation_level_); auto const route_to_leader = true; // write impl_ = std::make_shared( std::move(selector), route_to_leader, diff --git a/google/cloud/spanner/transaction.h b/google/cloud/spanner/transaction.h index 0949d52229a7f..a7bff177dd5ee 100644 --- a/google/cloud/spanner/transaction.h +++ b/google/cloud/spanner/transaction.h @@ -18,6 +18,7 @@ #include "google/cloud/spanner/internal/transaction_impl.h" #include "google/cloud/spanner/timestamp.h" #include "google/cloud/spanner/version.h" +#include "google/cloud/spanner/isolation_level.h" #include "absl/types/optional.h" #include #include @@ -103,10 +104,16 @@ class Transaction { // A tag used for collecting statistics about the transaction. ReadWriteOptions& WithTag(absl::optional tag); + ReadWriteOptions& WithIsolationLevel(IsolationLevel isolation_level) { + isolation_level_ = isolation_level; + return *this; + } + private: friend Transaction; google::spanner::v1::TransactionOptions_ReadWrite rw_opts_; absl::optional tag_; + IsolationLevel isolation_level_ = IsolationLevel::kUnspecified; }; /** diff --git a/google/cloud/spanner/transaction_test.cc b/google/cloud/spanner/transaction_test.cc index ac7a3b46483b5..e9cac370ff04d 100644 --- a/google/cloud/spanner/transaction_test.cc +++ b/google/cloud/spanner/transaction_test.cc @@ -14,6 +14,8 @@ #include "google/cloud/spanner/transaction.h" #include "google/cloud/spanner/internal/session.h" +#include "google/cloud/spanner/options.h" +#include "google/cloud/options.h" #include namespace google { @@ -169,6 +171,59 @@ TEST(Transaction, MultiplexedPreviousTransactionId) { }); } +TEST(Transaction, IsolationLevel) { + auto opts = Transaction::ReadWriteOptions().WithIsolationLevel( + IsolationLevel::kRepeatableRead); + Transaction txn = MakeReadWriteTransaction(opts); + spanner_internal::Visit( + txn, [](spanner_internal::SessionHolder&, + StatusOr& s, + spanner_internal::TransactionContext const&) { + EXPECT_TRUE(s->has_begin()); + EXPECT_TRUE(s->begin().has_read_write()); + EXPECT_EQ(s->begin().isolation_level(), + google::spanner::v1::TransactionOptions::REPEATABLE_READ); + + std::cout << "Isolation Level: " + << s->begin().isolation_level() << std::endl; + return 0; + }); +} + +TEST(Transaction, IsolationLevelPrecedence) { + internal::OptionsSpan span(Options{}.set( + IsolationLevel::kSerializable)); + + // Case 1: Per-call overrides client default + auto opts = Transaction::ReadWriteOptions().WithIsolationLevel( + IsolationLevel::kRepeatableRead); + Transaction txn = MakeReadWriteTransaction(opts); + spanner_internal::Visit( + txn, [](spanner_internal::SessionHolder&, + StatusOr& s, + spanner_internal::TransactionContext const&) { + EXPECT_EQ(s->begin().isolation_level(), + google::spanner::v1::TransactionOptions::REPEATABLE_READ); + std::cout << "Isolation Level: " + << s->begin().isolation_level() << std::endl; + return 0; + }); + + // Case 2: Fallback to client default + auto opts_default = Transaction::ReadWriteOptions(); + Transaction txn_default = MakeReadWriteTransaction(opts_default); + spanner_internal::Visit( + txn_default, [](spanner_internal::SessionHolder&, + StatusOr& s, + spanner_internal::TransactionContext const&) { + EXPECT_EQ(s->begin().isolation_level(), + google::spanner::v1::TransactionOptions::SERIALIZABLE); + std::cout << "Isolation Level: " + << s->begin().isolation_level() << std::endl; + return 0; + }); +} + TEST(Transaction, ReadWriteOptionsWithTag) { auto opts = Transaction::ReadWriteOptions().WithTag("test-tag"); Transaction txn = MakeReadWriteTransaction(opts); From 7451e09b18198141d28eea3cb67d644c15f4b8a4 Mon Sep 17 00:00:00 2001 From: Prathmesh Chandwade Date: Mon, 12 Jan 2026 17:26:38 +0000 Subject: [PATCH 2/3] spanner: Refine Transaction::ReadWriteOptions with IsolationLevel 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.# --- google/cloud/spanner/CMakeLists.txt | 1 - .../spanner/google_cloud_cpp_spanner.bzl | 1 - .../client_integration_test.cc | 17 ++++--- google/cloud/spanner/isolation_level.h | 43 ----------------- google/cloud/spanner/options.h | 17 ++++--- google/cloud/spanner/transaction.cc | 48 ++++++++++++++++--- google/cloud/spanner/transaction.h | 17 +++++-- google/cloud/spanner/transaction_test.cc | 29 +++++++---- 8 files changed, 90 insertions(+), 83 deletions(-) delete mode 100644 google/cloud/spanner/isolation_level.h diff --git a/google/cloud/spanner/CMakeLists.txt b/google/cloud/spanner/CMakeLists.txt index 8039d1c470809..acb029537fecb 100644 --- a/google/cloud/spanner/CMakeLists.txt +++ b/google/cloud/spanner/CMakeLists.txt @@ -173,7 +173,6 @@ add_library( internal/tuple_utils.h interval.cc interval.h - isolation_level.h json.h keys.cc keys.h diff --git a/google/cloud/spanner/google_cloud_cpp_spanner.bzl b/google/cloud/spanner/google_cloud_cpp_spanner.bzl index a2208d8e33ae4..2225628fa9d8e 100644 --- a/google/cloud/spanner/google_cloud_cpp_spanner.bzl +++ b/google/cloud/spanner/google_cloud_cpp_spanner.bzl @@ -94,7 +94,6 @@ google_cloud_cpp_spanner_hdrs = [ "internal/transaction_impl.h", "internal/tuple_utils.h", "interval.h", - "isolation_level.h", "json.h", "keys.h", "lock_hint.h", diff --git a/google/cloud/spanner/integration_tests/client_integration_test.cc b/google/cloud/spanner/integration_tests/client_integration_test.cc index e97139a2980d1..b267760bb2af3 100644 --- a/google/cloud/spanner/integration_tests/client_integration_test.cc +++ b/google/cloud/spanner/integration_tests/client_integration_test.cc @@ -16,7 +16,6 @@ #include "google/cloud/spanner/client.h" #include "google/cloud/spanner/database.h" #include "google/cloud/spanner/mutations.h" -#include "google/cloud/spanner/isolation_level.h" #include "google/cloud/spanner/options.h" #include "google/cloud/spanner/testing/database_integration_test.h" #include "google/cloud/credentials.h" @@ -241,6 +240,9 @@ TEST_F(ClientIntegrationTest, MultipleInserts) { RowType(4, "test-fname-4", "test-lname-4"))); } +// A smoke test to verify that the TransactionIsolationLevelOption is +// correctly processed and accepted by the Spanner backend. It does not +// test the behavioral semantics of the isolation level itself. /// @test Verify that TransactionIsolationLevel works as expected. TEST_F(ClientIntegrationTest, TransactionIsolationLevel) { auto& client = *client_; @@ -251,15 +253,13 @@ TEST_F(ClientIntegrationTest, TransactionIsolationLevel) { for (auto const& row : rows) { if (!row) return row.status(); } - // std::cout << "Transaction is active." << std::endl; return Mutations{}; }, - Options{}.set( - IsolationLevel::kRepeatableRead)); + Options{}.set( + Transaction::IsolationLevel::kRepeatableRead)); EXPECT_THAT(commit, StatusIs(StatusCode::kOk)); } - /// @test Verify that Client::Rollback works as expected. TEST_F(ClientIntegrationTest, TransactionRollback) { ASSERT_NO_FATAL_FAILURE(InsertTwoSingers()); @@ -1419,6 +1419,9 @@ TEST_F(PgClientIntegrationTest, FineGrainedAccessControl) { ASSERT_STATUS_OK(metadata); } +// A smoke test to verify that the TransactionIsolationLevelOption is +// correctly processed and accepted by the Spanner backend. It does not +// test the behavioral semantics of the isolation level itself. /// @test Verify that TransactionIsolationLevel works as expected. TEST_F(PgClientIntegrationTest, TransactionIsolationLevel) { auto& client = *client_; @@ -1430,8 +1433,8 @@ TEST_F(PgClientIntegrationTest, TransactionIsolationLevel) { } return Mutations{}; }, - Options{}.set( - IsolationLevel::kRepeatableRead)); + Options{}.set( + Transaction::IsolationLevel::kRepeatableRead)); EXPECT_THAT(commit, StatusIs(StatusCode::kOk)); } diff --git a/google/cloud/spanner/isolation_level.h b/google/cloud/spanner/isolation_level.h deleted file mode 100644 index 3648eac54709b..0000000000000 --- a/google/cloud/spanner/isolation_level.h +++ /dev/null @@ -1,43 +0,0 @@ -// google/cloud/spanner/isolation_level.h -// -// Copyright 2025 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// https://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_SPANNER_ISOLATION_LEVEL_H -#define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_SPANNER_ISOLATION_LEVEL_H - -#include "google/cloud/spanner/version.h" - -namespace google { -namespace cloud { -namespace spanner { -GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN - -/** - * The isolation level for read-write transactions. - * - * @see https://cloud.google.com/spanner/docs/reference/rpc/google.spanner.v1#google.spanner.v1.TransactionOptions.IsolationLevel - */ -enum class IsolationLevel { - kUnspecified = 0, - kSerializable = 1, - kRepeatableRead = 2, -}; - -GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END -} // namespace spanner -} // namespace cloud -} // namespace google - -#endif // GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_SPANNER_ISOLATION_LEVEL_H diff --git a/google/cloud/spanner/options.h b/google/cloud/spanner/options.h index 474f200f42b14..663b605ae17ce 100644 --- a/google/cloud/spanner/options.h +++ b/google/cloud/spanner/options.h @@ -41,12 +41,12 @@ #include "google/cloud/spanner/backoff_policy.h" #include "google/cloud/spanner/directed_read_replicas.h" #include "google/cloud/spanner/internal/session.h" -#include "google/cloud/spanner/isolation_level.h" #include "google/cloud/spanner/lock_hint.h" #include "google/cloud/spanner/order_by.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" #include "google/cloud/spanner/version.h" #include "google/cloud/options.h" #include "absl/types/variant.h" @@ -417,23 +417,22 @@ struct ExcludeTransactionFromChangeStreamsOption { }; /** - * Option for `google::cloud::Options` to return additional statistics - * about the committed transaction in a `spanner::CommitResult`. + * Option for `google::cloud::Options` to set the transaction isolation level. * * @ingroup google-cloud-spanner-options */ -struct CommitReturnStatsOption { - using Type = bool; +struct TransactionIsolationLevelOption { + using Type = spanner::Transaction::IsolationLevel; }; /** - * Option for `google::cloud::Options` to set the isolation level for - * read-write transactions. + * Option for `google::cloud::Options` to return additional statistics + * about the committed transaction in a `spanner::CommitResult`. * * @ingroup google-cloud-spanner-options */ -struct TransactionIsolationLevelOption { - using Type = IsolationLevel; +struct CommitReturnStatsOption { + using Type = bool; }; /** diff --git a/google/cloud/spanner/transaction.cc b/google/cloud/spanner/transaction.cc index aedffcf9dcbc2..a7c543f011e1b 100644 --- a/google/cloud/spanner/transaction.cc +++ b/google/cloud/spanner/transaction.cc @@ -54,6 +54,22 @@ ProtoReadLockMode( } } +google::spanner::v1::TransactionOptions_IsolationLevel ProtoIsolationLevel( + absl::optional const& isolation_level) { + if (!isolation_level) { + return google::spanner::v1::TransactionOptions::ISOLATION_LEVEL_UNSPECIFIED; + } + switch (*isolation_level) { + case Transaction::IsolationLevel::kSerializable: + return google::spanner::v1::TransactionOptions::SERIALIZABLE; + case Transaction::IsolationLevel::kRepeatableRead: + return google::spanner::v1::TransactionOptions::REPEATABLE_READ; + default: + return google::spanner::v1::TransactionOptions:: + ISOLATION_LEVEL_UNSPECIFIED; + } +} + google::spanner::v1::TransactionOptions MakeOpts( google::spanner::v1::TransactionOptions_ReadOnly ro_opts) { google::spanner::v1::TransactionOptions opts; @@ -63,21 +79,21 @@ google::spanner::v1::TransactionOptions MakeOpts( google::spanner::v1::TransactionOptions MakeOpts( google::spanner::v1::TransactionOptions_ReadWrite rw_opts, - IsolationLevel isolation_level) { + absl::optional isolation_level) { google::spanner::v1::TransactionOptions opts; *opts.mutable_read_write() = std::move(rw_opts); auto const& current = internal::CurrentOptions(); if (current.get()) { opts.set_exclude_txn_from_change_streams(true); } - if (isolation_level == IsolationLevel::kUnspecified) { - isolation_level = current.get(); - } - if (isolation_level != IsolationLevel::kUnspecified) { + if (isolation_level && + *isolation_level != Transaction::IsolationLevel::kUnspecified) { + opts.set_isolation_level(ProtoIsolationLevel(isolation_level)); + } else if (current.has()) { opts.set_isolation_level( - static_cast( - isolation_level)); + ProtoIsolationLevel(current.get())); } + return opts; } @@ -112,6 +128,13 @@ Transaction::ReadWriteOptions& Transaction::ReadWriteOptions::WithTag( return *this; } +Transaction::ReadWriteOptions& +Transaction::ReadWriteOptions::WithIsolationLevel( + IsolationLevel isolation_level) { + isolation_level_ = isolation_level; + return *this; +} + Transaction::SingleUseOptions::SingleUseOptions(ReadOnlyOptions opts) { ro_opts_ = std::move(opts.ro_opts_); } @@ -146,6 +169,17 @@ Transaction::Transaction(ReadWriteOptions opts) { std::move(opts.tag_).value_or(std::string())); } +Transaction::Transaction(ReadWriteOptions opts, + IsolationLevel isolation_level) { + google::spanner::v1::TransactionSelector selector; + *selector.mutable_begin() = + MakeOpts(std::move(opts.rw_opts_), isolation_level); + auto const route_to_leader = true; // read-write + impl_ = std::make_shared( + std::move(selector), route_to_leader, + std::move(opts.tag_).value_or(std::string())); +} + Transaction::Transaction(Transaction const& txn, ReadWriteOptions opts) { google::spanner::v1::TransactionSelector selector; *selector.mutable_begin() = diff --git a/google/cloud/spanner/transaction.h b/google/cloud/spanner/transaction.h index a7bff177dd5ee..43a07b06a2271 100644 --- a/google/cloud/spanner/transaction.h +++ b/google/cloud/spanner/transaction.h @@ -18,7 +18,6 @@ #include "google/cloud/spanner/internal/transaction_impl.h" #include "google/cloud/spanner/timestamp.h" #include "google/cloud/spanner/version.h" -#include "google/cloud/spanner/isolation_level.h" #include "absl/types/optional.h" #include #include @@ -58,6 +57,12 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN */ class Transaction { public: + enum class IsolationLevel { + kUnspecified, + kSerializable, + kRepeatableRead, + }; + /** * Options for ReadOnly transactions. */ @@ -101,13 +106,13 @@ class Transaction { explicit ReadWriteOptions(ReadLockMode read_lock_mode); + explicit ReadWriteOptions(IsolationLevel isolation_level) + : isolation_level_(isolation_level) {} + // A tag used for collecting statistics about the transaction. ReadWriteOptions& WithTag(absl::optional tag); - ReadWriteOptions& WithIsolationLevel(IsolationLevel isolation_level) { - isolation_level_ = isolation_level; - return *this; - } + ReadWriteOptions& WithIsolationLevel(IsolationLevel isolation_level); private: friend Transaction; @@ -157,6 +162,8 @@ class Transaction { /// @copydoc Transaction(ReadOnlyOptions) explicit Transaction(ReadWriteOptions opts); /// @copydoc Transaction(ReadOnlyOptions) + explicit Transaction(ReadWriteOptions opts, IsolationLevel isolation_level); + /// @copydoc Transaction(ReadOnlyOptions) Transaction(Transaction const& txn, ReadWriteOptions opts); ///@} diff --git a/google/cloud/spanner/transaction_test.cc b/google/cloud/spanner/transaction_test.cc index e9cac370ff04d..3823dd09ade43 100644 --- a/google/cloud/spanner/transaction_test.cc +++ b/google/cloud/spanner/transaction_test.cc @@ -173,7 +173,7 @@ TEST(Transaction, MultiplexedPreviousTransactionId) { TEST(Transaction, IsolationLevel) { auto opts = Transaction::ReadWriteOptions().WithIsolationLevel( - IsolationLevel::kRepeatableRead); + Transaction::IsolationLevel::kRepeatableRead); Transaction txn = MakeReadWriteTransaction(opts); spanner_internal::Visit( txn, [](spanner_internal::SessionHolder&, @@ -183,20 +183,17 @@ TEST(Transaction, IsolationLevel) { EXPECT_TRUE(s->begin().has_read_write()); EXPECT_EQ(s->begin().isolation_level(), google::spanner::v1::TransactionOptions::REPEATABLE_READ); - - std::cout << "Isolation Level: " - << s->begin().isolation_level() << std::endl; return 0; }); } TEST(Transaction, IsolationLevelPrecedence) { internal::OptionsSpan span(Options{}.set( - IsolationLevel::kSerializable)); + Transaction::IsolationLevel::kSerializable)); // Case 1: Per-call overrides client default auto opts = Transaction::ReadWriteOptions().WithIsolationLevel( - IsolationLevel::kRepeatableRead); + Transaction::IsolationLevel::kRepeatableRead); Transaction txn = MakeReadWriteTransaction(opts); spanner_internal::Visit( txn, [](spanner_internal::SessionHolder&, @@ -204,8 +201,6 @@ TEST(Transaction, IsolationLevelPrecedence) { spanner_internal::TransactionContext const&) { EXPECT_EQ(s->begin().isolation_level(), google::spanner::v1::TransactionOptions::REPEATABLE_READ); - std::cout << "Isolation Level: " - << s->begin().isolation_level() << std::endl; return 0; }); @@ -218,8 +213,6 @@ TEST(Transaction, IsolationLevelPrecedence) { spanner_internal::TransactionContext const&) { EXPECT_EQ(s->begin().isolation_level(), google::spanner::v1::TransactionOptions::SERIALIZABLE); - std::cout << "Isolation Level: " - << s->begin().isolation_level() << std::endl; return 0; }); } @@ -265,6 +258,22 @@ TEST(Transaction, ReadWriteOptionsWithReadLockMode) { google::spanner::v1::TransactionOptions_ReadWrite::OPTIMISTIC); } +TEST(Transaction, ReadWriteOptionsWithIsolationLevel) { + auto opts = Transaction::ReadWriteOptions( + Transaction::IsolationLevel::kRepeatableRead); + Transaction txn(opts, Transaction::IsolationLevel::kRepeatableRead); + spanner_internal::Visit( + txn, [](spanner_internal::SessionHolder&, + StatusOr& s, + spanner_internal::TransactionContext const&) { + EXPECT_TRUE(s->has_begin()); + EXPECT_TRUE(s->begin().has_read_write()); + EXPECT_EQ(s->begin().isolation_level(), + google::spanner::v1::TransactionOptions::REPEATABLE_READ); + return 0; + }); +} + } // namespace GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END } // namespace spanner From 72ec605f32bfec34c909f3432ad1233d4842c9e6 Mon Sep 17 00:00:00 2001 From: Prathmesh Chandwade Date: Wed, 14 Jan 2026 15:31:35 +0000 Subject: [PATCH 3/3] feat(spanner): refine Transaction::ReadWriteOptions with IsolationLevel This commit refines 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. These changes address reviewer feedback, enforcing that isolation levels are configured solely within the context of read-write transactions. This enhances type safe and improves the clarity of the client library's API. Key changes include: - Removing the `IsolationLevel` parameter from `Transaction` constructors, as it is now configured via `ReadWriteOptions`. - Enhancing `Transaction::ReadWriteOptions` with a `WithIsolationLevel` method, allowing for a fluent configuration of the desired isolation level (e.g., `kSerializable`, `kRepeatableRead`). - Updating internal transaction handling logic to correctly propagate the selected isolation level from `ReadWriteOptions` to the Spanner backend. --- .../client_integration_test.cc | 39 ------------------- google/cloud/spanner/transaction.cc | 11 ------ google/cloud/spanner/transaction.h | 30 +++++++++++++- google/cloud/spanner/transaction_test.cc | 32 --------------- 4 files changed, 28 insertions(+), 84 deletions(-) diff --git a/google/cloud/spanner/integration_tests/client_integration_test.cc b/google/cloud/spanner/integration_tests/client_integration_test.cc index b267760bb2af3..2c114ca56f18b 100644 --- a/google/cloud/spanner/integration_tests/client_integration_test.cc +++ b/google/cloud/spanner/integration_tests/client_integration_test.cc @@ -240,26 +240,6 @@ TEST_F(ClientIntegrationTest, MultipleInserts) { RowType(4, "test-fname-4", "test-lname-4"))); } -// A smoke test to verify that the TransactionIsolationLevelOption is -// correctly processed and accepted by the Spanner backend. It does not -// test the behavioral semantics of the isolation level itself. -/// @test Verify that TransactionIsolationLevel works as expected. -TEST_F(ClientIntegrationTest, TransactionIsolationLevel) { - auto& client = *client_; - auto commit = client.Commit( - [&](Transaction const& txn) -> StatusOr { - // Perform a read to ensure the transaction is active on the server. - auto rows = client.ExecuteQuery(txn, SqlStatement("SELECT 1")); - for (auto const& row : rows) { - if (!row) return row.status(); - } - return Mutations{}; - }, - Options{}.set( - Transaction::IsolationLevel::kRepeatableRead)); - EXPECT_THAT(commit, StatusIs(StatusCode::kOk)); -} - /// @test Verify that Client::Rollback works as expected. TEST_F(ClientIntegrationTest, TransactionRollback) { ASSERT_NO_FATAL_FAILURE(InsertTwoSingers()); @@ -1419,25 +1399,6 @@ TEST_F(PgClientIntegrationTest, FineGrainedAccessControl) { ASSERT_STATUS_OK(metadata); } -// A smoke test to verify that the TransactionIsolationLevelOption is -// correctly processed and accepted by the Spanner backend. It does not -// test the behavioral semantics of the isolation level itself. -/// @test Verify that TransactionIsolationLevel works as expected. -TEST_F(PgClientIntegrationTest, TransactionIsolationLevel) { - auto& client = *client_; - auto commit = client.Commit( - [&](Transaction const& txn) -> StatusOr { - auto rows = client.ExecuteQuery(txn, SqlStatement("SELECT 1")); - for (auto const& row : rows) { - if (!row) return row.status(); - } - return Mutations{}; - }, - Options{}.set( - Transaction::IsolationLevel::kRepeatableRead)); - EXPECT_THAT(commit, StatusIs(StatusCode::kOk)); -} - /// @test Verify "FOREIGN KEY" "ON DELETE CASCADE". TEST_F(ClientIntegrationTest, ForeignKeyDeleteCascade) { spanner_admin::DatabaseAdminClient admin_client( diff --git a/google/cloud/spanner/transaction.cc b/google/cloud/spanner/transaction.cc index a7c543f011e1b..457b21c943c76 100644 --- a/google/cloud/spanner/transaction.cc +++ b/google/cloud/spanner/transaction.cc @@ -169,17 +169,6 @@ Transaction::Transaction(ReadWriteOptions opts) { std::move(opts.tag_).value_or(std::string())); } -Transaction::Transaction(ReadWriteOptions opts, - IsolationLevel isolation_level) { - google::spanner::v1::TransactionSelector selector; - *selector.mutable_begin() = - MakeOpts(std::move(opts.rw_opts_), isolation_level); - auto const route_to_leader = true; // read-write - impl_ = std::make_shared( - std::move(selector), route_to_leader, - std::move(opts.tag_).value_or(std::string())); -} - Transaction::Transaction(Transaction const& txn, ReadWriteOptions opts) { google::spanner::v1::TransactionSelector selector; *selector.mutable_begin() = diff --git a/google/cloud/spanner/transaction.h b/google/cloud/spanner/transaction.h index 43a07b06a2271..2af203cba6fb0 100644 --- a/google/cloud/spanner/transaction.h +++ b/google/cloud/spanner/transaction.h @@ -57,9 +57,33 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN */ class Transaction { public: + /** + * Defines the isolation level for a transaction. + * + * This determines how concurrent transactions interact with each other and + * what consistency guarantees are provided for read and write operations. + * **Note:** This setting only applies to read-write transactions. + * + * See the `v1::TransactionOptions` proto for more details. + * + * @see https://docs.cloud.google.com/spanner/docs/isolation-levels + */ enum class IsolationLevel { + /// The isolation level is not specified, using the backend default. kUnspecified, + /** + * All transactions appear as if they executed in a serial order. + * This is the default isolation level for read-write transactions. + */ kSerializable, + /** + * All reads performed during the transaction observe a consistent snapshot + * of the database. The transaction is only successfully committed in the + * absence of conflicts between its updates and any concurrent updates + * that have occurred since that snapshot. Consequently, in contrast to + * `kSerializable` transactions, only write-write conflicts are detected in + * snapshot transactions. + */ kRepeatableRead, }; @@ -112,6 +136,10 @@ class Transaction { // A tag used for collecting statistics about the transaction. ReadWriteOptions& WithTag(absl::optional tag); + // Sets the isolation level for the transaction. This controls how the + // transaction interacts with other concurrent transactions, primarily + // regarding data consistency for reads and writes. + // See `IsolationLevel` enum for possible values. ReadWriteOptions& WithIsolationLevel(IsolationLevel isolation_level); private: @@ -162,8 +190,6 @@ class Transaction { /// @copydoc Transaction(ReadOnlyOptions) explicit Transaction(ReadWriteOptions opts); /// @copydoc Transaction(ReadOnlyOptions) - explicit Transaction(ReadWriteOptions opts, IsolationLevel isolation_level); - /// @copydoc Transaction(ReadOnlyOptions) Transaction(Transaction const& txn, ReadWriteOptions opts); ///@} diff --git a/google/cloud/spanner/transaction_test.cc b/google/cloud/spanner/transaction_test.cc index 3823dd09ade43..c57f2fc7fcc55 100644 --- a/google/cloud/spanner/transaction_test.cc +++ b/google/cloud/spanner/transaction_test.cc @@ -171,22 +171,6 @@ TEST(Transaction, MultiplexedPreviousTransactionId) { }); } -TEST(Transaction, IsolationLevel) { - auto opts = Transaction::ReadWriteOptions().WithIsolationLevel( - Transaction::IsolationLevel::kRepeatableRead); - Transaction txn = MakeReadWriteTransaction(opts); - spanner_internal::Visit( - txn, [](spanner_internal::SessionHolder&, - StatusOr& s, - spanner_internal::TransactionContext const&) { - EXPECT_TRUE(s->has_begin()); - EXPECT_TRUE(s->begin().has_read_write()); - EXPECT_EQ(s->begin().isolation_level(), - google::spanner::v1::TransactionOptions::REPEATABLE_READ); - return 0; - }); -} - TEST(Transaction, IsolationLevelPrecedence) { internal::OptionsSpan span(Options{}.set( Transaction::IsolationLevel::kSerializable)); @@ -258,22 +242,6 @@ TEST(Transaction, ReadWriteOptionsWithReadLockMode) { google::spanner::v1::TransactionOptions_ReadWrite::OPTIMISTIC); } -TEST(Transaction, ReadWriteOptionsWithIsolationLevel) { - auto opts = Transaction::ReadWriteOptions( - Transaction::IsolationLevel::kRepeatableRead); - Transaction txn(opts, Transaction::IsolationLevel::kRepeatableRead); - spanner_internal::Visit( - txn, [](spanner_internal::SessionHolder&, - StatusOr& s, - spanner_internal::TransactionContext const&) { - EXPECT_TRUE(s->has_begin()); - EXPECT_TRUE(s->begin().has_read_write()); - EXPECT_EQ(s->begin().isolation_level(), - google::spanner::v1::TransactionOptions::REPEATABLE_READ); - return 0; - }); -} - } // namespace GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END } // namespace spanner