From ff7e0fc105bc03f18993456b15d5d18faae16142 Mon Sep 17 00:00:00 2001 From: Navarre Ginsberg Date: Sun, 7 May 2023 14:39:21 -0700 Subject: [PATCH 1/7] Add batching delay to commit request options. --- google/cloud/spanner/commit_options.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/google/cloud/spanner/commit_options.h b/google/cloud/spanner/commit_options.h index c47df0b7f665c..a375d43336b1e 100644 --- a/google/cloud/spanner/commit_options.h +++ b/google/cloud/spanner/commit_options.h @@ -70,6 +70,16 @@ class CommitOptions { return request_priority_; } + CommitOptions& set_batching_delay( + absl::optional batching_delay) { + batching_delay_ = std::move(batching_delay); + return *this; + } + + absl::optional batching_delay() const { + return batching_delay_; + } + /** * Set the transaction tag for the `spanner::Client::Commit()` call. * Ignored for the overload that already takes a `spanner::Transaction`. @@ -90,6 +100,7 @@ class CommitOptions { // so we do not even provide a mechanism to specify one. bool return_stats_ = false; absl::optional request_priority_; + absl::optional batching_delay_; absl::optional transaction_tag_; }; From 6211d897a8cf84956e20a39908ed23c3361119d6 Mon Sep 17 00:00:00 2001 From: Navarre Ginsberg Date: Sun, 7 May 2023 14:52:47 -0700 Subject: [PATCH 2/7] add change to connection_impl.cc --- google/cloud/spanner/internal/connection_impl.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/google/cloud/spanner/internal/connection_impl.cc b/google/cloud/spanner/internal/connection_impl.cc index a27c38b3a4e47..9914750ee39ed 100644 --- a/google/cloud/spanner/internal/connection_impl.cc +++ b/google/cloud/spanner/internal/connection_impl.cc @@ -1017,6 +1017,10 @@ StatusOr ConnectionImpl::CommitImpl( request.mutable_request_options()->set_priority( ProtoRequestPriority(params.options.request_priority())); + if (params.options.batching_delay().has_value()) { + request.mutable_request_options()->set_batching_delay_ms( + absl::ToDoubleMillis(params.options.batching_delay.value())); + // params.options.transaction_tag() was either already used to set // ctx.tag (for a library-generated transaction), or it is ignored // (for a user-supplied transaction). From f94e59eeba28665a0518705e71ee986f7bc86b99 Mon Sep 17 00:00:00 2001 From: Navarre Ginsberg Date: Wed, 10 May 2023 14:40:39 -0700 Subject: [PATCH 3/7] Add code sample --- google/cloud/spanner/samples/samples.cc | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/google/cloud/spanner/samples/samples.cc b/google/cloud/spanner/samples/samples.cc index 921e4bb96952b..1075222f79876 100644 --- a/google/cloud/spanner/samples/samples.cc +++ b/google/cloud/spanner/samples/samples.cc @@ -3802,6 +3802,16 @@ void QueryInformationSchemaDatabaseOptions( } // [END spanner_query_information_schema_database_options] +// [START spanner_set_batching_delay] +void CommitWithBatchingDelay(google::cloud::spanner::Client client, + google::cloud::spanner::Mutations mutations, + absl::Duration batching_delay) { + google::cloud::spanner::CommitOptions options; + auto result = client.Commit(mutations, options); + if (!result) throw std::move(result).status(); +} +// [END spanner_set_batching_delay] + std::string Basename(absl::string_view name) { auto last_sep = name.find_last_of("/\\"); if (last_sep != absl::string_view::npos) name.remove_prefix(last_sep + 1); From ac533c7ea949355594b7c7b6752d2ebc6f99fe9e Mon Sep 17 00:00:00 2001 From: Navarre Ginsberg Date: Wed, 21 Jun 2023 11:00:04 -0700 Subject: [PATCH 4/7] Update --- google/cloud/spanner/commit_options.h | 12 ++++++------ google/cloud/spanner/internal/connection_impl.cc | 8 +++++--- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/google/cloud/spanner/commit_options.h b/google/cloud/spanner/commit_options.h index a375d43336b1e..ef7f22e57c0cd 100644 --- a/google/cloud/spanner/commit_options.h +++ b/google/cloud/spanner/commit_options.h @@ -70,14 +70,14 @@ class CommitOptions { return request_priority_; } - CommitOptions& set_batching_delay( - absl::optional batching_delay) { - batching_delay_ = std::move(batching_delay); + CommitOptions& set_max_batching_delay( + absl::optional max_batching_delay) { + max_batching_delay_ = std::move(max_batching_delay); return *this; } - absl::optional batching_delay() const { - return batching_delay_; + absl::optional max_batching_delay() const { + return max_batching_delay_; } /** @@ -100,7 +100,7 @@ class CommitOptions { // so we do not even provide a mechanism to specify one. bool return_stats_ = false; absl::optional request_priority_; - absl::optional batching_delay_; + absl::optional max_batching_delay_; absl::optional transaction_tag_; }; diff --git a/google/cloud/spanner/internal/connection_impl.cc b/google/cloud/spanner/internal/connection_impl.cc index 9914750ee39ed..21689e2ba57c9 100644 --- a/google/cloud/spanner/internal/connection_impl.cc +++ b/google/cloud/spanner/internal/connection_impl.cc @@ -1017,9 +1017,11 @@ StatusOr ConnectionImpl::CommitImpl( request.mutable_request_options()->set_priority( ProtoRequestPriority(params.options.request_priority())); - if (params.options.batching_delay().has_value()) { - request.mutable_request_options()->set_batching_delay_ms( - absl::ToDoubleMillis(params.options.batching_delay.value())); + if (params.options.max_batching_delay().has_value()) { + *request.mutable_max_batching_delay() = + util_time::EncodeGoogleApiProto( + params.options.max_batching_delay.value()).value(); + } // params.options.transaction_tag() was either already used to set // ctx.tag (for a library-generated transaction), or it is ignored From 05211883654c23bc1865b053d08c077de6bcad4c Mon Sep 17 00:00:00 2001 From: Navarre Ginsberg Date: Wed, 21 Jun 2023 23:15:07 +0000 Subject: [PATCH 5/7] builds --- google/cloud/spanner/commit_options.h | 1 + google/cloud/spanner/internal/connection_impl.cc | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/google/cloud/spanner/commit_options.h b/google/cloud/spanner/commit_options.h index ef7f22e57c0cd..ee8fa164d19c7 100644 --- a/google/cloud/spanner/commit_options.h +++ b/google/cloud/spanner/commit_options.h @@ -19,6 +19,7 @@ #include "google/cloud/spanner/version.h" #include "google/cloud/options.h" #include "absl/types/optional.h" +#include "absl/time/time.h" #include namespace google { diff --git a/google/cloud/spanner/internal/connection_impl.cc b/google/cloud/spanner/internal/connection_impl.cc index 21689e2ba57c9..21ce1de81278e 100644 --- a/google/cloud/spanner/internal/connection_impl.cc +++ b/google/cloud/spanner/internal/connection_impl.cc @@ -1019,8 +1019,8 @@ StatusOr ConnectionImpl::CommitImpl( if (params.options.max_batching_delay().has_value()) { *request.mutable_max_batching_delay() = - util_time::EncodeGoogleApiProto( - params.options.max_batching_delay.value()).value(); + google::protobuf::util::TimeUtil::MillisecondsToDuration( + absl::ToDoubleMilliseconds(params.options.max_batching_delay().value())); } // params.options.transaction_tag() was either already used to set From 14940b6059cab73416f8a1911bd686a08a968a8f Mon Sep 17 00:00:00 2001 From: Navarre Ginsberg Date: Wed, 26 Jul 2023 15:49:21 -0700 Subject: [PATCH 6/7] Reviewer comments --- google/cloud/spanner/commit_options.cc | 3 +++ google/cloud/spanner/commit_options.h | 12 ++++----- .../cloud/spanner/internal/connection_impl.cc | 4 +-- google/cloud/spanner/options.h | 9 +++++++ google/cloud/spanner/samples/samples.cc | 27 ++++++++++++++----- 5 files changed, 40 insertions(+), 15 deletions(-) diff --git a/google/cloud/spanner/commit_options.cc b/google/cloud/spanner/commit_options.cc index da09fd7ea296e..707216167c956 100644 --- a/google/cloud/spanner/commit_options.cc +++ b/google/cloud/spanner/commit_options.cc @@ -30,6 +30,9 @@ CommitOptions::CommitOptions(Options const& opts) if (opts.has()) { transaction_tag_ = opts.get(); } + if (opts.has()) { + max_batching_delay_ms_ = opts.get(); + } } CommitOptions::operator Options() const { diff --git a/google/cloud/spanner/commit_options.h b/google/cloud/spanner/commit_options.h index ee8fa164d19c7..45ba2b6abbc4b 100644 --- a/google/cloud/spanner/commit_options.h +++ b/google/cloud/spanner/commit_options.h @@ -71,14 +71,14 @@ class CommitOptions { return request_priority_; } - CommitOptions& set_max_batching_delay( - absl::optional max_batching_delay) { - max_batching_delay_ = std::move(max_batching_delay); + CommitOptions& set_max_batching_delay_ms( + int max_batching_delay_ms) { + max_batching_delay_ms_ = std::move(max_batching_delay_ms); return *this; } - absl::optional max_batching_delay() const { - return max_batching_delay_; + absl::optional max_batching_delay_ms() const { + return max_batching_delay_ms; } /** @@ -101,7 +101,7 @@ class CommitOptions { // so we do not even provide a mechanism to specify one. bool return_stats_ = false; absl::optional request_priority_; - absl::optional max_batching_delay_; + absl::optional max_batching_delay_ms_; absl::optional transaction_tag_; }; diff --git a/google/cloud/spanner/internal/connection_impl.cc b/google/cloud/spanner/internal/connection_impl.cc index 587ae1a77f335..e0f2346b006a7 100644 --- a/google/cloud/spanner/internal/connection_impl.cc +++ b/google/cloud/spanner/internal/connection_impl.cc @@ -1025,10 +1025,10 @@ StatusOr ConnectionImpl::CommitImpl( request.mutable_request_options()->set_priority( ProtoRequestPriority(params.options.request_priority())); - if (params.options.max_batching_delay().has_value()) { + if (params.options.has()) { *request.mutable_max_batching_delay() = google::protobuf::util::TimeUtil::MillisecondsToDuration( - absl::ToDoubleMilliseconds(params.options.max_batching_delay().value())); + params.options.get()); } // params.options.transaction_tag() was either already used to set diff --git a/google/cloud/spanner/options.h b/google/cloud/spanner/options.h index ddf1d1f64491f..3d8044024eb67 100644 --- a/google/cloud/spanner/options.h +++ b/google/cloud/spanner/options.h @@ -232,6 +232,15 @@ struct RequestPriorityOption { using Type = spanner::RequestPriority; }; +/** + * Option for `google::cloud::Options` to set a maximum batching delay. + * + * @ingroup spanner-options + */ +struct MaxBatchingDelayMsOption { + using Type = int; +}; + /** * Option for `google::cloud::Options` to set a per-request tag. * diff --git a/google/cloud/spanner/samples/samples.cc b/google/cloud/spanner/samples/samples.cc index 25b8f9810ee6e..023cd12d27529 100644 --- a/google/cloud/spanner/samples/samples.cc +++ b/google/cloud/spanner/samples/samples.cc @@ -3951,13 +3951,19 @@ void QueryInformationSchemaDatabaseOptions( } // [END spanner_query_information_schema_database_options] -// [START spanner_set_batching_delay] -void CommitWithBatchingDelay(google::cloud::spanner::Client client, - google::cloud::spanner::Mutations mutations, - absl::Duration batching_delay) { - google::cloud::spanner::CommitOptions options; - auto result = client.Commit(mutations, options); - if (!result) throw std::move(result).status(); +// [START spanner_commit_with_batching_delay] +void UpdateDataWithBatchingDelay(google::cloud::spanner::Client client) { + namespace spanner = ::google::cloud::spanner; + Options ops; + ops.set(100); + auto commit_result = client.Commit(spanner::Mutations{ + spanner::UpdateMutationBuilder("Albums", + {"SingerId", "AlbumId", "MarketingBudget"}) + .EmplaceRow(1, 1, 100000) + .EmplaceRow(2, 2, 500000) + .Build()}, ops); + if (!commit_result) throw std::move(commit_result).status(); + std::cout << "Update was successful [spanner_update_data]\n"; } // [END spanner_set_batching_delay] @@ -4157,6 +4163,8 @@ int RunOneCommand(std::vector argv) { make_command_entry("make-delete-mutation", MakeDeleteMutation), make_command_entry("query-information-schema-database-options", QueryInformationSchemaDatabaseOptions), + make_command_entry("spanner-commit-with-batching-delay", + UpdateDataWithBatchingDelay), }; static std::string usage_msg = [&argv, &commands] { @@ -4879,6 +4887,11 @@ void RunAll(bool emulator) { QueryInformationSchemaDatabaseOptions(client); } + if (!emulator) { + SampleBanner("spanner_commit_with_batching_delay"); + UpdateDataWithBatchingDelay(client); + } + if (!emulator) { SampleBanner("spanner_create_table_with_foreign_key_delete_cascade"); CreateTableWithForeignKeyDeleteCascade(database_admin_client, project_id, From 9bd2e5673194dab6e84a6a8ceb2552ebcbec37ca Mon Sep 17 00:00:00 2001 From: Navarre Ginsberg Date: Wed, 26 Jul 2023 15:50:44 -0700 Subject: [PATCH 7/7] Remove unneeded include --- google/cloud/spanner/commit_options.h | 1 - 1 file changed, 1 deletion(-) diff --git a/google/cloud/spanner/commit_options.h b/google/cloud/spanner/commit_options.h index 45ba2b6abbc4b..e50b77c0d1f45 100644 --- a/google/cloud/spanner/commit_options.h +++ b/google/cloud/spanner/commit_options.h @@ -19,7 +19,6 @@ #include "google/cloud/spanner/version.h" #include "google/cloud/options.h" #include "absl/types/optional.h" -#include "absl/time/time.h" #include namespace google {