From d2cbc486038c249632dd7b57a67c7b2fc5422937 Mon Sep 17 00:00:00 2001 From: Bradley White <14679271+devbww@users.noreply.github.com> Date: Fri, 3 Nov 2023 01:16:07 -0400 Subject: [PATCH 1/2] feat(spanner): control replicas/regions used in non-transactional reads Add the `DirectedReadOption` to indicate which replicas or regions should be used for `Client::Read()`, `Client::ExecuteQuery()`, and `Client::ProfileQuery()` calls in read-only or single-use transactions. - The `IncludeReplicas` variant lists the replicas to try (in order) to process the request, and what to do if the list is exhausted without finding a healthy replica. - The `ExcludeReplicas` variant lists replicas that should be excluded from serving the request. --- google/cloud/spanner/CMakeLists.txt | 1 + google/cloud/spanner/client.cc | 127 +++++++++++------- google/cloud/spanner/client_test.cc | 90 +++++++++---- google/cloud/spanner/connection.h | 3 + google/cloud/spanner/directed_read_replicas.h | 127 ++++++++++++++++++ .../spanner/google_cloud_cpp_spanner.bzl | 1 + .../cloud/spanner/internal/connection_impl.cc | 73 +++++++++- .../spanner/internal/connection_impl_test.cc | 121 +++++++++++++++-- google/cloud/spanner/options.h | 19 +++ google/cloud/spanner/query_partition.h | 16 ++- google/cloud/spanner/query_partition_test.cc | 10 +- google/cloud/spanner/read_partition.h | 12 +- google/cloud/spanner/read_partition_test.cc | 16 ++- google/cloud/spanner/samples/samples.cc | 54 +++++++- 14 files changed, 562 insertions(+), 108 deletions(-) create mode 100644 google/cloud/spanner/directed_read_replicas.h diff --git a/google/cloud/spanner/CMakeLists.txt b/google/cloud/spanner/CMakeLists.txt index 94ff5d14afff4..8cf3575200e51 100644 --- a/google/cloud/spanner/CMakeLists.txt +++ b/google/cloud/spanner/CMakeLists.txt @@ -113,6 +113,7 @@ add_library( database_admin_connection.cc database_admin_connection.h date.h + directed_read_replicas.h encryption_config.h iam_updater.h instance.cc diff --git a/google/cloud/spanner/client.cc b/google/cloud/spanner/client.cc index 0a0d9cd38c70e..535ad0f6a429e 100644 --- a/google/cloud/spanner/client.cc +++ b/google/cloud/spanner/client.cc @@ -35,7 +35,8 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN namespace { -// Extracts an option value as an `absl::optional`. +// Returns an option value as an `absl::optional`. If `OptionType` is +// not present in `opts`, the returned optional is empty (disengaged). template absl::optional OptOpt(Options const& opts) { absl::optional optopt; @@ -43,41 +44,58 @@ absl::optional OptOpt(Options const& opts) { return optopt; } +// Extracts (removes and returns) an option value from `opts`. If +// `OptionType` is not present, returns a default-constructed value. +template +typename OptionType::Type ExtractOpt(Options& opts) { + auto option = internal::ExtractOption(opts); + return option ? *std::move(option) : typename OptionType::Type(); +} + } // namespace RowStream Client::Read(std::string table, KeySet keys, std::vector columns, Options opts) { - internal::OptionsSpan span(internal::MergeOptions(std::move(opts), opts_)); + opts = internal::MergeOptions(std::move(opts), opts_); + auto directed_read_option = ExtractOpt(opts); + internal::OptionsSpan span(std::move(opts)); return conn_->Read({spanner_internal::MakeSingleUseTransaction( Transaction::ReadOnlyOptions()), std::move(table), std::move(keys), std::move(columns), - ToReadOptions(internal::CurrentOptions()), - absl::nullopt}); + ToReadOptions(internal::CurrentOptions()), absl::nullopt, + false, std::move(directed_read_option)}); } RowStream Client::Read(Transaction::SingleUseOptions transaction_options, std::string table, KeySet keys, std::vector columns, Options opts) { - internal::OptionsSpan span(internal::MergeOptions(std::move(opts), opts_)); + opts = internal::MergeOptions(std::move(opts), opts_); + auto directed_read_option = ExtractOpt(opts); + internal::OptionsSpan span(std::move(opts)); return conn_->Read({spanner_internal::MakeSingleUseTransaction( std::move(transaction_options)), std::move(table), std::move(keys), std::move(columns), - ToReadOptions(internal::CurrentOptions()), - absl::nullopt}); + ToReadOptions(internal::CurrentOptions()), absl::nullopt, + false, std::move(directed_read_option)}); } RowStream Client::Read(Transaction transaction, std::string table, KeySet keys, std::vector columns, Options opts) { - internal::OptionsSpan span(internal::MergeOptions(std::move(opts), opts_)); + opts = internal::MergeOptions(std::move(opts), opts_); + auto directed_read_option = ExtractOpt(opts); + internal::OptionsSpan span(std::move(opts)); return conn_->Read({std::move(transaction), std::move(table), std::move(keys), std::move(columns), - ToReadOptions(internal::CurrentOptions()), - absl::nullopt}); + ToReadOptions(internal::CurrentOptions()), absl::nullopt, + false, std::move(directed_read_option)}); } RowStream Client::Read(ReadPartition const& read_partition, Options opts) { - internal::OptionsSpan span(internal::MergeOptions(std::move(opts), opts_)); - return conn_->Read(spanner_internal::MakeReadParams(read_partition)); + opts = internal::MergeOptions(std::move(opts), opts_); + auto directed_read_option = ExtractOpt(opts); + internal::OptionsSpan span(std::move(opts)); + return conn_->Read(spanner_internal::MakeReadParams( + read_partition, std::move(directed_read_option))); } StatusOr> Client::PartitionRead( @@ -87,70 +105,87 @@ StatusOr> Client::PartitionRead( return conn_->PartitionRead( {{std::move(transaction), std::move(table), std::move(keys), std::move(columns), ToReadOptions(internal::CurrentOptions()), - absl::nullopt}, + absl::nullopt, false, DirectedReadOption::Type{}}, ToPartitionOptions(internal::CurrentOptions())}); } RowStream Client::ExecuteQuery(SqlStatement statement, Options opts) { - internal::OptionsSpan span(internal::MergeOptions(std::move(opts), opts_)); - return conn_->ExecuteQuery({spanner_internal::MakeSingleUseTransaction( - Transaction::ReadOnlyOptions()), - std::move(statement), - QueryOptions(internal::CurrentOptions()), - absl::nullopt}); + opts = internal::MergeOptions(std::move(opts), opts_); + auto directed_read_option = ExtractOpt(opts); + internal::OptionsSpan span(std::move(opts)); + return conn_->ExecuteQuery( + {spanner_internal::MakeSingleUseTransaction( + Transaction::ReadOnlyOptions()), + std::move(statement), QueryOptions(internal::CurrentOptions()), + absl::nullopt, false, std::move(directed_read_option)}); } RowStream Client::ExecuteQuery( Transaction::SingleUseOptions transaction_options, SqlStatement statement, Options opts) { - internal::OptionsSpan span(internal::MergeOptions(std::move(opts), opts_)); - return conn_->ExecuteQuery({spanner_internal::MakeSingleUseTransaction( - std::move(transaction_options)), - std::move(statement), - QueryOptions(internal::CurrentOptions()), - absl::nullopt}); + opts = internal::MergeOptions(std::move(opts), opts_); + auto directed_read_option = ExtractOpt(opts); + internal::OptionsSpan span(std::move(opts)); + return conn_->ExecuteQuery( + {spanner_internal::MakeSingleUseTransaction( + std::move(transaction_options)), + std::move(statement), QueryOptions(internal::CurrentOptions()), + absl::nullopt, false, std::move(directed_read_option)}); } RowStream Client::ExecuteQuery(Transaction transaction, SqlStatement statement, Options opts) { - internal::OptionsSpan span(internal::MergeOptions(std::move(opts), opts_)); + opts = internal::MergeOptions(std::move(opts), opts_); + auto directed_read_option = ExtractOpt(opts); + internal::OptionsSpan span(std::move(opts)); return conn_->ExecuteQuery({std::move(transaction), std::move(statement), QueryOptions(internal::CurrentOptions()), - absl::nullopt}); + absl::nullopt, false, + std::move(directed_read_option)}); } RowStream Client::ExecuteQuery(QueryPartition const& partition, Options opts) { - internal::OptionsSpan span(internal::MergeOptions(std::move(opts), opts_)); + opts = internal::MergeOptions(std::move(opts), opts_); + auto directed_read_option = ExtractOpt(opts); + internal::OptionsSpan span(std::move(opts)); return conn_->ExecuteQuery(spanner_internal::MakeSqlParams( - partition, QueryOptions(internal::CurrentOptions()))); + partition, QueryOptions(internal::CurrentOptions()), + std::move(directed_read_option))); } ProfileQueryResult Client::ProfileQuery(SqlStatement statement, Options opts) { - internal::OptionsSpan span(internal::MergeOptions(std::move(opts), opts_)); - return conn_->ProfileQuery({spanner_internal::MakeSingleUseTransaction( - Transaction::ReadOnlyOptions()), - std::move(statement), - QueryOptions(internal::CurrentOptions()), - absl::nullopt}); + opts = internal::MergeOptions(std::move(opts), opts_); + auto directed_read_option = ExtractOpt(opts); + internal::OptionsSpan span(std::move(opts)); + return conn_->ProfileQuery( + {spanner_internal::MakeSingleUseTransaction( + Transaction::ReadOnlyOptions()), + std::move(statement), QueryOptions(internal::CurrentOptions()), + absl::nullopt, false, std::move(directed_read_option)}); } ProfileQueryResult Client::ProfileQuery( Transaction::SingleUseOptions transaction_options, SqlStatement statement, Options opts) { - internal::OptionsSpan span(internal::MergeOptions(std::move(opts), opts_)); - return conn_->ProfileQuery({spanner_internal::MakeSingleUseTransaction( - std::move(transaction_options)), - std::move(statement), - QueryOptions(internal::CurrentOptions()), - absl::nullopt}); + opts = internal::MergeOptions(std::move(opts), opts_); + auto directed_read_option = ExtractOpt(opts); + internal::OptionsSpan span(std::move(opts)); + return conn_->ProfileQuery( + {spanner_internal::MakeSingleUseTransaction( + std::move(transaction_options)), + std::move(statement), QueryOptions(internal::CurrentOptions()), + absl::nullopt, false, std::move(directed_read_option)}); } ProfileQueryResult Client::ProfileQuery(Transaction transaction, SqlStatement statement, Options opts) { - internal::OptionsSpan span(internal::MergeOptions(std::move(opts), opts_)); + opts = internal::MergeOptions(std::move(opts), opts_); + auto directed_read_option = ExtractOpt(opts); + internal::OptionsSpan span(std::move(opts)); return conn_->ProfileQuery({std::move(transaction), std::move(statement), QueryOptions(internal::CurrentOptions()), - absl::nullopt}); + absl::nullopt, false, + std::move(directed_read_option)}); } StatusOr> Client::PartitionQuery( @@ -166,7 +201,7 @@ StatusOr Client::ExecuteDml(Transaction transaction, internal::OptionsSpan span(internal::MergeOptions(std::move(opts), opts_)); return conn_->ExecuteDml({std::move(transaction), std::move(statement), QueryOptions(internal::CurrentOptions()), - absl::nullopt}); + absl::nullopt, false, DirectedReadOption::Type{}}); } StatusOr Client::ProfileDml(Transaction transaction, @@ -175,7 +210,7 @@ StatusOr Client::ProfileDml(Transaction transaction, internal::OptionsSpan span(internal::MergeOptions(std::move(opts), opts_)); return conn_->ProfileDml({std::move(transaction), std::move(statement), QueryOptions(internal::CurrentOptions()), - absl::nullopt}); + absl::nullopt, false, DirectedReadOption::Type{}}); } StatusOr Client::AnalyzeSql(Transaction transaction, @@ -184,7 +219,7 @@ StatusOr Client::AnalyzeSql(Transaction transaction, internal::OptionsSpan span(internal::MergeOptions(std::move(opts), opts_)); return conn_->AnalyzeSql({std::move(transaction), std::move(statement), QueryOptions(internal::CurrentOptions()), - absl::nullopt}); + absl::nullopt, false, DirectedReadOption::Type{}}); } StatusOr Client::ExecuteBatchDml( diff --git a/google/cloud/spanner/client_test.cc b/google/cloud/spanner/client_test.cc index b5791bb4987c3..d147ac5f4f3f3 100644 --- a/google/cloud/spanner/client_test.cc +++ b/google/cloud/spanner/client_test.cc @@ -57,10 +57,12 @@ using ::testing::Eq; using ::testing::Field; using ::testing::HasSubstr; using ::testing::Pair; +using ::testing::Property; using ::testing::Return; using ::testing::SaveArg; using ::testing::SizeIs; using ::testing::UnorderedElementsAre; +using ::testing::VariantWith; TEST(ClientTest, CopyAndMove) { auto conn1 = std::make_shared(); @@ -91,7 +93,6 @@ TEST(ClientTest, ReadSuccess) { auto conn = std::make_shared(); Client client(conn); - auto source = std::make_unique(); auto constexpr kText = R"pb( row_type: { fields: { @@ -106,17 +107,29 @@ TEST(ClientTest, ReadSuccess) { )pb"; google::spanner::v1::ResultSetMetadata metadata; ASSERT_TRUE(TextFormat::ParseFromString(kText, &metadata)); - EXPECT_CALL(*source, Metadata()).WillRepeatedly(Return(metadata)); - EXPECT_CALL(*source, NextRow()) - .WillOnce(Return(spanner_mocks::MakeRow("Steve", 12))) - .WillOnce(Return(spanner_mocks::MakeRow("Ann", 42))) - .WillOnce(Return(Row())); EXPECT_CALL(*conn, Read) - .WillOnce(Return(ByMove(RowStream(std::move(source))))); + .WillOnce([&metadata](Connection::ReadParams const& params) { + EXPECT_THAT( + params.directed_read_option, + VariantWith(AllOf( + Property(&IncludeReplicas::replica_selections, + ElementsAre(ReplicaSelection(ReplicaType::kReadOnly))), + Property(&IncludeReplicas::auto_failover_disabled, true)))); + auto source = std::make_unique(); + EXPECT_CALL(*source, Metadata()).WillRepeatedly(Return(metadata)); + EXPECT_CALL(*source, NextRow()) + .WillOnce(Return(spanner_mocks::MakeRow("Steve", 12))) + .WillOnce(Return(spanner_mocks::MakeRow("Ann", 42))) + .WillOnce(Return(Row())); + return RowStream(std::move(source)); + }); KeySet keys = KeySet::All(); - auto rows = client.Read("table", std::move(keys), {"column1", "column2"}); + auto rows = client.Read("table", std::move(keys), {"column1", "column2"}, + Options{}.set(IncludeReplicas( + {ReplicaSelection(ReplicaType::kReadOnly)}, + /*auto_failover_disabled=*/true))); using RowType = std::tuple; auto stream = StreamOf(rows); @@ -171,7 +184,6 @@ TEST(ClientTest, ExecuteQuerySuccess) { auto conn = std::make_shared(); Client client(conn); - auto source = std::make_unique(); auto constexpr kText = R"pb( row_type: { fields: { @@ -186,17 +198,30 @@ TEST(ClientTest, ExecuteQuerySuccess) { )pb"; google::spanner::v1::ResultSetMetadata metadata; ASSERT_TRUE(TextFormat::ParseFromString(kText, &metadata)); - EXPECT_CALL(*source, Metadata()).WillRepeatedly(Return(metadata)); - EXPECT_CALL(*source, NextRow()) - .WillOnce(Return(spanner_mocks::MakeRow("Steve", 12))) - .WillOnce(Return(spanner_mocks::MakeRow("Ann", 42))) - .WillOnce(Return(Row())); EXPECT_CALL(*conn, ExecuteQuery) - .WillOnce(Return(ByMove(RowStream(std::move(source))))); + .WillOnce([&metadata](Connection::SqlParams const& params) { + EXPECT_THAT( + params.directed_read_option, + VariantWith(AllOf( + Property(&IncludeReplicas::replica_selections, + ElementsAre(ReplicaSelection("us-east4"))), + Property(&IncludeReplicas::auto_failover_disabled, false)))); + auto source = std::make_unique(); + EXPECT_CALL(*source, Metadata()).WillRepeatedly(Return(metadata)); + EXPECT_CALL(*source, NextRow()) + .WillOnce(Return(spanner_mocks::MakeRow("Steve", 12))) + .WillOnce(Return(spanner_mocks::MakeRow("Ann", 42))) + .WillOnce(Return(Row())); + return RowStream(std::move(source)); + }); KeySet keys = KeySet::All(); - auto rows = client.ExecuteQuery(SqlStatement("SELECT * FROM Table;")); + auto rows = + client.ExecuteQuery(SqlStatement("SELECT * FROM Table;"), + Options{}.set(IncludeReplicas( + {ReplicaSelection("us-east4")}, + /*auto_failover_disabled=*/false))); using RowType = std::tuple; auto stream = StreamOf(rows); @@ -379,7 +404,7 @@ TEST(ClientTest, CommitMutatorSuccess) { auto conn = std::make_shared(); Transaction txn = MakeReadWriteTransaction(); // placeholder - Connection::ReadParams actual_read_params{txn, {}, {}, {}, {}, {}}; + Connection::ReadParams actual_read_params{txn, {}, {}, {}, {}, {}, {}, {}}; Connection::CommitParams actual_commit_params{txn, {}, {}}; auto source = std::make_unique(); @@ -428,7 +453,7 @@ TEST(ClientTest, CommitMutatorSuccess) { TEST(ClientTest, CommitMutatorRollback) { auto conn = std::make_shared(); Transaction txn = MakeReadWriteTransaction(); // placeholder - Connection::ReadParams actual_read_params{txn, {}, {}, {}, {}, {}}; + Connection::ReadParams actual_read_params{txn, {}, {}, {}, {}, {}, {}, {}}; auto source = std::make_unique(); auto constexpr kText = R"pb( @@ -470,7 +495,7 @@ TEST(ClientTest, CommitMutatorRollback) { TEST(ClientTest, CommitMutatorRollbackError) { auto conn = std::make_shared(); Transaction txn = MakeReadWriteTransaction(); // placeholder - Connection::ReadParams actual_read_params{txn, {}, {}, {}, {}, {}}; + Connection::ReadParams actual_read_params{txn, {}, {}, {}, {}, {}, {}, {}}; auto source = std::make_unique(); auto constexpr kText = R"pb( @@ -1063,7 +1088,6 @@ TEST(ClientTest, ProfileQuerySuccess) { auto conn = std::make_shared(); Client client(conn); - auto source = std::make_unique(); auto constexpr kText0 = R"pb( row_type: { fields: { @@ -1089,17 +1113,29 @@ TEST(ClientTest, ProfileQuerySuccess) { )pb"; google::spanner::v1::ResultSetStats stats; ASSERT_TRUE(TextFormat::ParseFromString(kText1, &stats)); - EXPECT_CALL(*source, Metadata()).WillRepeatedly(Return(metadata)); - EXPECT_CALL(*source, NextRow()) - .WillOnce(Return(spanner_mocks::MakeRow("Ann", 42))) - .WillOnce(Return(Row())); - EXPECT_CALL(*source, Stats()).WillRepeatedly(Return(stats)); EXPECT_CALL(*conn, ProfileQuery) - .WillOnce(Return(ByMove(ProfileQueryResult(std::move(source))))); + .WillOnce([&metadata, &stats](Connection::SqlParams const& params) { + EXPECT_THAT(params.directed_read_option, + VariantWith(Property( + &ExcludeReplicas::replica_selections, + ElementsAre(ReplicaSelection(ReplicaType::kReadWrite), + ReplicaSelection("us-east4"))))); + auto source = std::make_unique(); + EXPECT_CALL(*source, Metadata()).WillRepeatedly(Return(metadata)); + EXPECT_CALL(*source, NextRow()) + .WillOnce(Return(spanner_mocks::MakeRow("Ann", 42))) + .WillOnce(Return(Row())); + EXPECT_CALL(*source, Stats()).WillRepeatedly(Return(stats)); + return ProfileQueryResult(std::move(source)); + }); KeySet keys = KeySet::All(); - auto rows = client.ProfileQuery(SqlStatement("SELECT * FROM Table;")); + auto rows = client.ProfileQuery( + SqlStatement("SELECT * FROM Table;"), + Options{}.set( + ExcludeReplicas({ReplicaSelection(ReplicaType::kReadWrite), + ReplicaSelection("us-east4")}))); using RowType = std::tuple; auto stream = StreamOf(rows); diff --git a/google/cloud/spanner/connection.h b/google/cloud/spanner/connection.h index 7a746af7846aa..766974d94ed69 100644 --- a/google/cloud/spanner/connection.h +++ b/google/cloud/spanner/connection.h @@ -20,6 +20,7 @@ #include "google/cloud/spanner/commit_result.h" #include "google/cloud/spanner/keys.h" #include "google/cloud/spanner/mutations.h" +#include "google/cloud/spanner/options.h" #include "google/cloud/spanner/partition_options.h" #include "google/cloud/spanner/partitioned_dml_result.h" #include "google/cloud/spanner/query_options.h" @@ -79,6 +80,7 @@ class Connection { ReadOptions read_options; absl::optional partition_token; bool partition_data_boost = false; // when partition_token + DirectedReadOption::Type directed_read_option; }; /// Wrap the arguments to `PartitionRead()`. @@ -95,6 +97,7 @@ class Connection { QueryOptions query_options; absl::optional partition_token; bool partition_data_boost = false; // when partition_token + DirectedReadOption::Type directed_read_option; }; /// Wrap the arguments to `ExecutePartitionedDml()`. diff --git a/google/cloud/spanner/directed_read_replicas.h b/google/cloud/spanner/directed_read_replicas.h new file mode 100644 index 0000000000000..d0f35a63e21a0 --- /dev/null +++ b/google/cloud/spanner/directed_read_replicas.h @@ -0,0 +1,127 @@ +// Copyright 2023 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_DIRECTED_READ_REPLICAS_H +#define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_SPANNER_DIRECTED_READ_REPLICAS_H + +#include "google/cloud/spanner/version.h" +#include "absl/types/optional.h" +#include +#include +#include +#include + +namespace google { +namespace cloud { +namespace spanner { +GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN + +/** + * Indicates the type of replica. + */ +enum class ReplicaType { + kReadWrite, // Read-write replicas support both reads and writes. + kReadOnly, // Read-only replicas only support reads (not writes). +}; + +/** + * The directed-read replica selector. + * + * Callers must provide one or more of the following fields: + * - location: One of the regions within the multi-region configuration + * of your database. + * - type: The type of the replica. + */ +class ReplicaSelection { + public: + // Only replicas in the location and of the given type will be used + // to process the request. + ReplicaSelection(std::string location, ReplicaType type) + : location_(std::move(location)), type_(type) {} + + // Replicas in the location, of any available type, will be used to + // process the request. + explicit ReplicaSelection(std::string location) + : location_(std::move(location)), type_(absl::nullopt) {} + + // Replicas of the given type, in the nearest available location, will + // be used to process the request. + explicit ReplicaSelection(ReplicaType type) + : location_(absl::nullopt), type_(type) {} + + absl::optional const& location() const { return location_; } + absl::optional const& type() const { return type_; } + + private: + absl::optional location_; + absl::optional type_; +}; + +inline bool operator==(ReplicaSelection const& a, ReplicaSelection const& b) { + return a.location() == b.location() && a.type() == b.type(); +} + +inline bool operator!=(ReplicaSelection const& a, ReplicaSelection const& b) { + return !(a == b); +} + +/** + * An `IncludeReplicas` contains an ordered list of `ReplicaSelection`s + * that should be considered when serving requests. + * + * When `auto_failover_disabled` is set, requests will NOT be routed to + * a healthy replica outside the list when all replicas in the list are + * unavailable/unhealthy. + */ +class IncludeReplicas { + public: + IncludeReplicas(std::initializer_list replica_selections, + bool auto_failover_disabled = false) + : replica_selections_(replica_selections), + auto_failover_disabled_(auto_failover_disabled) {} + + std::vector const& replica_selections() const { + return replica_selections_; + } + bool auto_failover_disabled() const { return auto_failover_disabled_; } + + private: + std::vector replica_selections_; + bool auto_failover_disabled_; +}; + +/** + * An `ExcludeReplicas` contains a list of `ReplicaSelection`s that should + * be excluded from serving requests. + */ +class ExcludeReplicas { + public: + ExcludeReplicas(std::initializer_list replica_selections) + : replica_selections_(replica_selections.begin(), + replica_selections.end()) {} + + std::vector const& replica_selections() const { + return replica_selections_; + } + + private: + std::vector replica_selections_; +}; + +GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END +} // namespace spanner +} // namespace cloud +} // namespace google + +#endif // GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_SPANNER_DIRECTED_READ_REPLICAS_H diff --git a/google/cloud/spanner/google_cloud_cpp_spanner.bzl b/google/cloud/spanner/google_cloud_cpp_spanner.bzl index a5cabcfd50272..8b7082e898c6e 100644 --- a/google/cloud/spanner/google_cloud_cpp_spanner.bzl +++ b/google/cloud/spanner/google_cloud_cpp_spanner.bzl @@ -61,6 +61,7 @@ google_cloud_cpp_spanner_hdrs = [ "database_admin_client.h", "database_admin_connection.h", "date.h", + "directed_read_replicas.h", "encryption_config.h", "iam_updater.h", "instance.h", diff --git a/google/cloud/spanner/internal/connection_impl.cc b/google/cloud/spanner/internal/connection_impl.cc index 005b106657628..882e0ecd830d7 100644 --- a/google/cloud/spanner/internal/connection_impl.cc +++ b/google/cloud/spanner/internal/connection_impl.cc @@ -33,6 +33,7 @@ #include #include #include +#include namespace google { namespace cloud { @@ -41,6 +42,57 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN namespace { +class DirectedReadVisitor { + public: + explicit DirectedReadVisitor( + std::function factory) + : factory_(std::move(factory)) {} + + void operator()(absl::monostate) const { + // No inclusions/exclusions. + } + + void operator()(spanner::IncludeReplicas const& replicas) const { + auto* include_replicas = factory_()->mutable_include_replicas(); + for (auto const& replica_selection : replicas.replica_selections()) { + ToProto(replica_selection, include_replicas->add_replica_selections()); + } + if (replicas.auto_failover_disabled()) { + include_replicas->set_auto_failover_disabled(true); + } + } + + void operator()(spanner::ExcludeReplicas const& replicas) const { + auto* exclude_replicas = factory_()->mutable_exclude_replicas(); + for (auto const& replica_selection : replicas.replica_selections()) { + ToProto(replica_selection, exclude_replicas->add_replica_selections()); + } + } + + private: + static void ToProto( + spanner::ReplicaSelection const& from, + google::spanner::v1::DirectedReadOptions::ReplicaSelection* to) { + if (auto location = from.location()) { + to->set_location(*location); + } + if (auto type = from.type()) { + switch (*type) { + case spanner::ReplicaType::kReadWrite: + to->set_type(google::spanner::v1::DirectedReadOptions:: + ReplicaSelection::READ_WRITE); + break; + case spanner::ReplicaType::kReadOnly: + to->set_type(google::spanner::v1::DirectedReadOptions:: + ReplicaSelection::READ_ONLY); + break; + } + } + } + + std::function factory_; +}; + inline std::shared_ptr const& RetryPolicyPrototype() { return internal::CurrentOptions().get(); } @@ -494,6 +546,10 @@ spanner::RowStream ConnectionImpl::ReadImpl( *std::move(params.read_options.request_tag)); } request->mutable_request_options()->set_transaction_tag(ctx.tag); + absl::visit(DirectedReadVisitor([&request] { + return request->mutable_directed_read_options(); + }), + params.directed_read_option); // Capture a copy of `stub` to ensure the `shared_ptr<>` remains valid through // the lifetime of the lambda. @@ -676,6 +732,10 @@ StatusOr ConnectionImpl::ExecuteSqlImpl( *params.query_options.request_tag()); } request.mutable_request_options()->set_transaction_tag(ctx.tag); + absl::visit(DirectedReadVisitor([&request] { + return request.mutable_directed_read_options(); + }), + params.directed_read_option); for (;;) { auto reader = retry_resume_fn(request); @@ -1030,11 +1090,14 @@ ConnectionImpl::ExecutePartitionedDmlImpl( } s->set_id(begin->id()); - SqlParams sql_params( - {MakeTransactionFromIds(session->session_name(), begin->id(), - ctx.route_to_leader, ctx.tag), - std::move(params.statement), std::move(params.query_options), - /*partition_token=*/{}}); + SqlParams sql_params{ + MakeTransactionFromIds(session->session_name(), begin->id(), + ctx.route_to_leader, ctx.tag), + std::move(params.statement), + std::move(params.query_options), + /*partition_token=*/absl::nullopt, + /*partition_data_boost=*/false, + spanner::DirectedReadOption::Type{}}; auto dml_result = CommonQueryImpl( session, s, ctx, std::move(sql_params), google::spanner::v1::ExecuteSqlRequest::NORMAL); diff --git a/google/cloud/spanner/internal/connection_impl_test.cc b/google/cloud/spanner/internal/connection_impl_test.cc index 84b8bf6ae1d7b..b0cefbb8a0273 100644 --- a/google/cloud/spanner/internal/connection_impl_test.cc +++ b/google/cloud/spanner/internal/connection_impl_test.cc @@ -127,41 +127,40 @@ class ProtoBuilder { }; // Matchers for mock calls. -MATCHER_P(HasSession, session, "request has expected session name") { +MATCHER_P(HasSession, session, "has expected session name") { return arg.session() == session; } -MATCHER_P(HasTransactionId, transaction_id, - "request has expected transaction id") { +MATCHER_P(HasTransactionId, transaction_id, "has expected transaction id") { return arg.transaction().id() == transaction_id; } // As above, but for Commit and Rollback requests, which don't have a // `TransactionSelector` but just store the "naked" ID directly in the proto. MATCHER_P(HasNakedTransactionId, transaction_id, - "commit or rollback request has expected transaction id") { + "has expected transaction id") { return arg.transaction_id() == transaction_id; } MATCHER_P(HasReturnStats, return_commit_stats, - "commit request has expected return-stats value") { + "has expected return-stats value") { return arg.return_commit_stats() == return_commit_stats; } -MATCHER(HasBeginTransaction, "request has begin TransactionSelector set") { +MATCHER(HasBeginTransaction, "has begin TransactionSelector set") { return arg.transaction().has_begin(); } -MATCHER_P(HasDatabase, database, "request has expected database") { +MATCHER_P(HasDatabase, database, "has expected database") { return arg.database() == database.FullName(); } -MATCHER_P(HasCreatorRole, role, "request has expected creator role") { +MATCHER_P(HasCreatorRole, role, "has expected creator role") { return arg.session_template().creator_role() == role; } // Matches a `spanner::Transaction` that is bound to a "bad" session. -MATCHER(HasBadSession, "bound to a session that's marked bad") { +MATCHER(HasBadSession, "is bound to a session that's marked bad") { return Visit(arg, [&](SessionHolder& session, StatusOr&, TransactionContext const&) { @@ -177,18 +176,26 @@ MATCHER(HasBadSession, "bound to a session that's marked bad") { }); } -MATCHER_P(HasPriority, priority, "request has expected priority") { +MATCHER_P(HasPriority, priority, "has expected priority") { return arg.request_options().priority() == priority; } -MATCHER_P(HasRequestTag, tag, "request has expected request tag") { +MATCHER_P(HasRequestTag, tag, "has expected request tag") { return arg.request_options().request_tag() == tag; } -MATCHER_P(HasTransactionTag, tag, "request has expected transaction tag") { +MATCHER_P(HasTransactionTag, tag, "has expected transaction tag") { return arg.request_options().transaction_tag() == tag; } +MATCHER_P(HasReplicaLocation, location, "has expected replica location") { + return arg.location() == location; +} + +MATCHER_P(HasReplicaType, type, "has expected replica type") { + return arg.type() == type; +} + // Ideally this would be a matcher, but matcher args are `const` and `RowStream` // only has non-const methods. bool ContainsNoRows(spanner::RowStream& rows) { @@ -471,6 +478,51 @@ TEST(ConnectionImplTest, ReadSuccess) { IsOkAndHolds(RowType(42, "Ann")))); } +TEST(ConnectionImplTest, ReadDirectedRead) { + auto mock = std::make_shared(); + auto db = spanner::Database("placeholder_project", "placeholder_instance", + "placeholder_database_id"); + EXPECT_CALL(*mock, BatchCreateSessions(_, HasDatabase(db))) + .WillOnce(Return(MakeSessionsResponse({"test-session-name"}))); + EXPECT_CALL(*mock, BeginTransaction).Times(0); + EXPECT_CALL(*mock, StreamingRead) + .WillOnce([](std::shared_ptr const&, + google::spanner::v1::ReadRequest const& request) { + EXPECT_EQ("test-session-name", request.session()); + EXPECT_TRUE(request.has_directed_read_options()); + auto const& directed_read_options = request.directed_read_options(); + EXPECT_TRUE(directed_read_options.has_include_replicas()); + EXPECT_THAT( + directed_read_options.include_replicas().replica_selections(), + ElementsAre( + HasReplicaLocation("us-east4"), + HasReplicaType(google::spanner::v1::DirectedReadOptions:: + ReplicaSelection::READ_ONLY))); + return MakeReader( + {R"pb(metadata: { transaction: { id: "ABCDEF00" } })pb"}); + }); + + auto conn = MakeConnectionImpl(db, mock); + internal::OptionsSpan span(MakeLimitedTimeOptions()); + spanner::Transaction txn = + MakeReadOnlyTransaction(spanner::Transaction::ReadOnlyOptions()); + auto rows = conn->Read( + {txn, + "table", + spanner::KeySet::All(), + {"UserId", "UserName"}, + spanner::ReadOptions{}, + /*partition_token=*/absl::nullopt, + /*partition_data_boost=*/false, + spanner::IncludeReplicas( + {spanner::ReplicaSelection("us-east4"), + spanner::ReplicaSelection(spanner::ReplicaType::kReadOnly)}, + true)}); + EXPECT_TRUE(ContainsNoRows(rows)); + EXPECT_THAT(txn, HasSessionAndTransaction("test-session-name", "ABCDEF00", + false, "")); +} + TEST(ConnectionImplTest, ReadPermanentFailure) { auto mock = std::make_shared(); auto db = spanner::Database("placeholder_project", "placeholder_instance", @@ -794,6 +846,47 @@ TEST(ConnectionImplTest, ExecuteQueryReadSuccess) { 42, "Ann", spanner::MakeNumeric(12345678, -2).value())))); } +TEST(ConnectionImplTest, ExecuteQueryDirectedRead) { + auto mock = std::make_shared(); + auto db = spanner::Database("placeholder_project", "placeholder_instance", + "placeholder_database_id"); + EXPECT_CALL(*mock, BatchCreateSessions(_, HasDatabase(db))) + .WillOnce(Return(MakeSessionsResponse({"test-session-name"}))); + EXPECT_CALL(*mock, BeginTransaction).Times(0); + EXPECT_CALL(*mock, ExecuteStreamingSql) + .WillOnce([](std::shared_ptr const&, + google::spanner::v1::ExecuteSqlRequest const& request) { + EXPECT_EQ("test-session-name", request.session()); + EXPECT_TRUE(request.has_directed_read_options()); + auto const& directed_read_options = request.directed_read_options(); + EXPECT_TRUE(directed_read_options.has_exclude_replicas()); + EXPECT_THAT( + directed_read_options.exclude_replicas().replica_selections(), + ElementsAre( + HasReplicaType(google::spanner::v1::DirectedReadOptions:: + ReplicaSelection::READ_WRITE), + HasReplicaLocation("us-east4"))); + return MakeReader( + {R"pb(metadata: { transaction: { id: "00FEDCBA" } })pb"}); + }); + + auto conn = MakeConnectionImpl(db, mock); + internal::OptionsSpan span(MakeLimitedTimeOptions()); + spanner::Transaction txn = + MakeReadOnlyTransaction(spanner::Transaction::ReadOnlyOptions()); + auto rows = conn->ExecuteQuery( + {txn, spanner::SqlStatement("SELECT * FROM Table"), + spanner::QueryOptions{}, + /*partition_token=*/absl::nullopt, + /*partition_data_boost=*/false, + spanner::ExcludeReplicas( + {spanner::ReplicaSelection(spanner::ReplicaType::kReadWrite), + spanner::ReplicaSelection("us-east4")})}); + EXPECT_TRUE(ContainsNoRows(rows)); + EXPECT_THAT(txn, HasSessionAndTransaction("test-session-name", "00FEDCBA", + false, "")); +} + TEST(ConnectionImplTest, ExecuteQueryPgNumericResult) { auto mock = std::make_shared(); auto db = spanner::Database("placeholder_project", "placeholder_instance", @@ -2665,7 +2758,7 @@ TEST(ConnectionImplTest, ReadPartition) { EXPECT_EQ("DEADBEEF", request.partition_token()); EXPECT_TRUE(request.data_boost_enabled()); return MakeReader( - {R"pb(metadata: { transaction: { id: " ABCDEF00 " } })pb"}); + {R"pb(metadata: { transaction: { id: "ABCDEF00" } })pb"}); }); auto conn = MakeConnectionImpl(db, mock); @@ -2820,7 +2913,7 @@ TEST(ConnectionImplTest, QueryPartition) { EXPECT_EQ("DEADBEEF", request.partition_token()); EXPECT_TRUE(request.data_boost_enabled()); return MakeReader( - {R"pb(metadata: { transaction: { id: " ABCDEF00 " } })pb"}); + {R"pb(metadata: { transaction: { id: "ABCDEF00" } })pb"}); }); auto conn = MakeConnectionImpl(db, mock); diff --git a/google/cloud/spanner/options.h b/google/cloud/spanner/options.h index d5f8f478d900f..9fe94ae722822 100644 --- a/google/cloud/spanner/options.h +++ b/google/cloud/spanner/options.h @@ -39,12 +39,14 @@ */ #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/polling_policy.h" #include "google/cloud/spanner/request_priority.h" #include "google/cloud/spanner/retry_policy.h" #include "google/cloud/spanner/version.h" #include "google/cloud/options.h" +#include "absl/types/variant.h" #include #include #include @@ -323,6 +325,23 @@ struct PartitionDataBoostOption { using Type = bool; }; +/** + * Option for `google::cloud::Options` to indicate which replicas or regions + * should be used for reads/queries in read-only or single-use transactions. + * Use of DirectedReadOptions within a read-write transaction will result in + * a `kInvalidArgument` error. + * + * The `IncludeReplicas` variant lists the replicas to try (in order) to + * process the request, and what to do if the list is exhausted without + * finding a healthy replica. + * + * Alternately, the `ExcludeReplicas` variant lists replicas that should + * be excluded from serving the request. + */ +struct DirectedReadOption { + using Type = absl::variant; +}; + /** * Option for `google::cloud::Options` to set a per-transaction tag. * diff --git a/google/cloud/spanner/query_partition.h b/google/cloud/spanner/query_partition.h index 5a4b0a75a9441..bfb9737477a66 100644 --- a/google/cloud/spanner/query_partition.h +++ b/google/cloud/spanner/query_partition.h @@ -165,13 +165,17 @@ struct QueryPartitionInternals { static spanner::Connection::SqlParams MakeSqlParams( spanner::QueryPartition const& query_partition, - spanner::QueryOptions const& query_options) { + spanner::QueryOptions const& query_options, + spanner::DirectedReadOption::Type directed_read_option) { return {MakeTransactionFromIds(query_partition.session_id(), query_partition.transaction_id(), query_partition.route_to_leader(), query_partition.transaction_tag()), - query_partition.sql_statement(), query_options, - query_partition.partition_token(), query_partition.data_boost()}; + query_partition.sql_statement(), + query_options, + query_partition.partition_token(), + query_partition.data_boost(), + std::move(directed_read_option)}; } }; @@ -187,8 +191,10 @@ inline spanner::QueryPartition MakeQueryPartition( inline spanner::Connection::SqlParams MakeSqlParams( spanner::QueryPartition const& query_partition, - spanner::QueryOptions const& query_options) { - return QueryPartitionInternals::MakeSqlParams(query_partition, query_options); + spanner::QueryOptions const& query_options, + spanner::DirectedReadOption::Type directed_read_option) { + return QueryPartitionInternals::MakeSqlParams( + query_partition, query_options, std::move(directed_read_option)); } GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END diff --git a/google/cloud/spanner/query_partition_test.cc b/google/cloud/spanner/query_partition_test.cc index 662db0ace61ed..3e6cc8a87936b 100644 --- a/google/cloud/spanner/query_partition_test.cc +++ b/google/cloud/spanner/query_partition_test.cc @@ -58,7 +58,10 @@ namespace { using ::google::cloud::spanner_internal::QueryPartitionTester; using ::google::cloud::spanner_testing::HasSessionAndTransaction; using ::google::cloud::testing_util::IsOk; +using ::testing::ElementsAre; using ::testing::Not; +using ::testing::Property; +using ::testing::VariantWith; TEST(QueryPartitionTest, MakeQueryPartition) { std::string stmt("SELECT * FROM foo WHERE name = @name"); @@ -151,7 +154,8 @@ TEST(QueryPartitionTest, MakeSqlParams) { Connection::SqlParams params = spanner_internal::MakeSqlParams( expected_partition.Partition(), - QueryOptions{}.set_request_tag("request_tag")); + QueryOptions{}.set_request_tag("request_tag"), + ExcludeReplicas({ReplicaSelection(ReplicaType::kReadWrite)})); EXPECT_EQ(params.statement, SqlStatement("SELECT * FROM foo WHERE name = @name", @@ -161,6 +165,10 @@ TEST(QueryPartitionTest, MakeSqlParams) { EXPECT_THAT(params.transaction, HasSessionAndTransaction("session", "txn-id", true, "tag")); EXPECT_EQ(*params.query_options.request_tag(), "request_tag"); + EXPECT_THAT(params.directed_read_option, + VariantWith(Property( + &ExcludeReplicas::replica_selections, + ElementsAre(ReplicaSelection(ReplicaType::kReadWrite))))); } } // namespace diff --git a/google/cloud/spanner/read_partition.h b/google/cloud/spanner/read_partition.h index 93152a4d8d2fe..0801d25f620fc 100644 --- a/google/cloud/spanner/read_partition.h +++ b/google/cloud/spanner/read_partition.h @@ -173,7 +173,8 @@ struct ReadPartitionInternals { } static spanner::Connection::ReadParams MakeReadParams( - spanner::ReadPartition const& read_partition) { + spanner::ReadPartition const& read_partition, + spanner::DirectedReadOption::Type directed_read_option) { return spanner::Connection::ReadParams{ MakeTransactionFromIds( read_partition.SessionId(), read_partition.TransactionId(), @@ -183,7 +184,8 @@ struct ReadPartitionInternals { read_partition.ColumnNames(), read_partition.ReadOptions(), read_partition.PartitionToken(), - read_partition.DataBoost()}; + read_partition.DataBoost(), + std::move(directed_read_option)}; } }; @@ -201,8 +203,10 @@ inline spanner::ReadPartition MakeReadPartition( } inline spanner::Connection::ReadParams MakeReadParams( - spanner::ReadPartition const& read_partition) { - return ReadPartitionInternals::MakeReadParams(read_partition); + spanner::ReadPartition const& read_partition, + spanner::DirectedReadOption::Type directed_read_option) { + return ReadPartitionInternals::MakeReadParams( + read_partition, std::move(directed_read_option)); } GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END diff --git a/google/cloud/spanner/read_partition_test.cc b/google/cloud/spanner/read_partition_test.cc index a90a002085993..ddb2d553eb533 100644 --- a/google/cloud/spanner/read_partition_test.cc +++ b/google/cloud/spanner/read_partition_test.cc @@ -58,7 +58,11 @@ using ::google::cloud::spanner_internal::ReadPartitionTester; using ::google::cloud::spanner_testing::HasSessionAndTransaction; using ::google::cloud::testing_util::IsOk; using ::google::cloud::testing_util::IsProtoEqual; +using ::testing::AllOf; +using ::testing::ElementsAre; using ::testing::Not; +using ::testing::Property; +using ::testing::VariantWith; TEST(ReadPartitionTest, MakeReadPartition) { std::string partition_token("token"); @@ -199,8 +203,10 @@ TEST(ReadPartitionTest, MakeReadParams) { "txn-id", true, "tag", "session", "token", "Students", KeySet::All(), columns, false, read_options)); - Connection::ReadParams params = - spanner_internal::MakeReadParams(expected_partition.Partition()); + Connection::ReadParams params = spanner_internal::MakeReadParams( + expected_partition.Partition(), + IncludeReplicas({ReplicaSelection(ReplicaType::kReadWrite)}, + /*auto_failover_disabled=*/true)); EXPECT_EQ(*params.partition_token, "token"); EXPECT_EQ(params.read_options, read_options); @@ -210,6 +216,12 @@ TEST(ReadPartitionTest, MakeReadParams) { EXPECT_EQ(params.table, "Students"); EXPECT_THAT(params.transaction, HasSessionAndTransaction("session", "txn-id", true, "tag")); + EXPECT_THAT( + params.directed_read_option, + VariantWith(AllOf( + Property(&IncludeReplicas::replica_selections, + ElementsAre(ReplicaSelection(ReplicaType::kReadWrite))), + Property(&IncludeReplicas::auto_failover_disabled, true)))); } } // namespace diff --git a/google/cloud/spanner/samples/samples.cc b/google/cloud/spanner/samples/samples.cc index fd955a0bedefa..27f56151b84e7 100644 --- a/google/cloud/spanner/samples/samples.cc +++ b/google/cloud/spanner/samples/samples.cc @@ -2578,6 +2578,48 @@ void SetRequestTag(google::cloud::spanner::Client client) { } //! [END spanner_set_request_tag] +//! [START spanner_directed_read] +void DirectedRead(std::string const& project_id, std::string const& instance_id, + std::string const& database_id) { + namespace spanner = ::google::cloud::spanner; + + // Create a client with a DirectedReadOption. + auto client = spanner::Client( + spanner::MakeConnection( + spanner::Database(project_id, instance_id, database_id)), + google::cloud::Options{}.set( + spanner::ExcludeReplicas({spanner::ReplicaSelection("us-east4")}))); + + spanner::SqlStatement select( + "SELECT SingerId, AlbumId, AlbumTitle FROM Albums"); + using RowType = std::tuple; + + // A DirectedReadOption on the operation will override the option set + // at the client level. + auto rows = client.ExecuteQuery( + std::move(select), + google::cloud::Options{}.set( + spanner::IncludeReplicas( + {spanner::ReplicaSelection(spanner::ReplicaType::kReadWrite)}, + /*auto_failover_disabled=*/true))); + for (auto& row : spanner::StreamOf(rows)) { + if (!row) throw std::move(row).status(); + std::cout << "SingerId: " << std::get<0>(*row) + << " AlbumId: " << std::get<1>(*row) + << " AlbumTitle: " << std::get<2>(*row) << "\n"; + } + std::cout << "Read completed for [spanner_directed_read]\n"; +} +//! [END spanner_directed_read] + +void DirectedReadCommand(std::vector argv) { + if (argv.size() != 3) { + throw std::runtime_error( + "directed-read "); + } + DirectedRead(argv[0], argv[1], argv[2]); +} + //! [START spanner_batch_client] void UsePartitionQuery(google::cloud::spanner::Client client) { namespace spanner = ::google::cloud::spanner; @@ -2709,8 +2751,9 @@ void QueryUsingIndex(google::cloud::spanner::Client client) { void CreateClientWithQueryOptions(std::string const& project_id, std::string const& instance_id, - std::string const& db_id) { - auto db = ::google::cloud::spanner::Database(project_id, instance_id, db_id); + std::string const& database_id) { + auto db = + ::google::cloud::spanner::Database(project_id, instance_id, database_id); //! [START spanner_create_client_with_query_options] namespace spanner = ::google::cloud::spanner; spanner::Client client( @@ -4152,10 +4195,9 @@ std::string Basename(absl::string_view name) { int RunOneCommand(std::vector argv) { using CommandType = std::function const&)>; + using CommandMap = std::map; using SampleFunction = void (*)(google::cloud::spanner::Client); - - using CommandMap = std::map; auto make_command_entry = [](std::string const& sample_name, SampleFunction sample) { auto make_command = [](std::string const& sample_name, @@ -4277,6 +4319,7 @@ int RunOneCommand(std::vector argv) { make_command_entry("set-transaction-tag", SetTransactionTag), make_command_entry("read-stale-data", ReadStaleData), make_command_entry("set-request-tag", SetRequestTag), + {"directed-read", DirectedReadCommand}, make_command_entry("use-partition-query", UsePartitionQuery), make_command_entry("read-data-with-index", ReadDataWithIndex), make_command_entry("query-new-column", QueryNewColumn), @@ -4887,6 +4930,9 @@ void RunAll(bool emulator) { SampleBanner("spanner_set_request_tag"); SetRequestTag(client); + SampleBanner("spanner_directed_read"); + DirectedRead(project_id, instance_id, database_id); + SampleBanner("spanner_batch_client"); UsePartitionQuery(client); From ef6c62813f52e8db7645743cbe9b5f1bb84b0276 Mon Sep 17 00:00:00 2001 From: Bradley White <14679271+devbww@users.noreply.github.com> Date: Fri, 3 Nov 2023 15:00:10 -0400 Subject: [PATCH 2/2] Address review comments --- .../cloud/spanner/internal/connection_impl.cc | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/google/cloud/spanner/internal/connection_impl.cc b/google/cloud/spanner/internal/connection_impl.cc index 882e0ecd830d7..4b2b5f78deed8 100644 --- a/google/cloud/spanner/internal/connection_impl.cc +++ b/google/cloud/spanner/internal/connection_impl.cc @@ -44,6 +44,8 @@ namespace { class DirectedReadVisitor { public: + // @p `factory` produces a mutable `DirectedReadOptions` proto should it be + // needed by a visitor to one of the `DirectedReadOption::Type` variants. explicit DirectedReadVisitor( std::function factory) : factory_(std::move(factory)) {} @@ -55,7 +57,7 @@ class DirectedReadVisitor { void operator()(spanner::IncludeReplicas const& replicas) const { auto* include_replicas = factory_()->mutable_include_replicas(); for (auto const& replica_selection : replicas.replica_selections()) { - ToProto(replica_selection, include_replicas->add_replica_selections()); + *include_replicas->add_replica_selections() = ToProto(replica_selection); } if (replicas.auto_failover_disabled()) { include_replicas->set_auto_failover_disabled(true); @@ -65,29 +67,30 @@ class DirectedReadVisitor { void operator()(spanner::ExcludeReplicas const& replicas) const { auto* exclude_replicas = factory_()->mutable_exclude_replicas(); for (auto const& replica_selection : replicas.replica_selections()) { - ToProto(replica_selection, exclude_replicas->add_replica_selections()); + *exclude_replicas->add_replica_selections() = ToProto(replica_selection); } } private: - static void ToProto( - spanner::ReplicaSelection const& from, - google::spanner::v1::DirectedReadOptions::ReplicaSelection* to) { + static google::spanner::v1::DirectedReadOptions::ReplicaSelection ToProto( + spanner::ReplicaSelection const& from) { + google::spanner::v1::DirectedReadOptions::ReplicaSelection proto; if (auto location = from.location()) { - to->set_location(*location); + proto.set_location(*location); } if (auto type = from.type()) { switch (*type) { case spanner::ReplicaType::kReadWrite: - to->set_type(google::spanner::v1::DirectedReadOptions:: - ReplicaSelection::READ_WRITE); + proto.set_type(google::spanner::v1::DirectedReadOptions:: + ReplicaSelection::READ_WRITE); break; case spanner::ReplicaType::kReadOnly: - to->set_type(google::spanner::v1::DirectedReadOptions:: - ReplicaSelection::READ_ONLY); + proto.set_type(google::spanner::v1::DirectedReadOptions:: + ReplicaSelection::READ_ONLY); break; } } + return proto; } std::function factory_;