feat(spanner): rollback CommitStats prior to Feb release (#5787)

This reverts #5745.

Author: devbww

ci/test-abi/google_cloud_cpp_spanner.expected.abi.dump.gz

@@ -44,7 +44,6 @@ add_library(
     client.cc
     client.h
     client_options.h
-    commit_options.h
     commit_result.h
     connection.h
     connection_options.cc
@@ -251,7 +250,6 @@ function (spanner_client_define_tests)
         bytes_test.cc
         client_options_test.cc
         client_test.cc
-        commit_options_test.cc
         connection_options_test.cc
         create_instance_request_builder_test.cc
         database_admin_client_test.cc

google/cloud/spanner/CMakeLists.txt

@@ -188,8 +188,7 @@ StatusOr<BatchDmlResult> Client::ExecuteBatchDml(
 StatusOr<CommitResult> Client::Commit(
     std::function<StatusOr<Mutations>(Transaction)> const& mutator,
     std::unique_ptr<TransactionRerunPolicy> rerun_policy,
-    std::unique_ptr<BackoffPolicy> backoff_policy,
-    CommitOptions const& options) {
+    std::unique_ptr<BackoffPolicy> backoff_policy) {
   // The status-code discriminator of TransactionRerunPolicy.
   using RerunnablePolicy = spanner_internal::SafeTransactionRerun;
 
@@ -219,7 +218,7 @@ StatusOr<CommitResult> Client::Commit(
 #endif
     auto status = mutations.status();
     if (RerunnablePolicy::IsOk(status)) {
-      auto result = Commit(txn, *mutations, options);
+      auto result = Commit(txn, *mutations);
       status = result.status();
       if (!RerunnablePolicy::IsTransientFailure(status)) {
         return result;
@@ -258,8 +257,7 @@ StatusOr<CommitResult> Client::Commit(
 }
 
 StatusOr<CommitResult> Client::Commit(
-    std::function<StatusOr<Mutations>(Transaction)> const& mutator,
-    CommitOptions const& options) {
+    std::function<StatusOr<Mutations>(Transaction)> const& mutator) {
   auto const rerun_maximum_duration = std::chrono::minutes(10);
   auto default_commit_rerun_policy =
       LimitedTimeTransactionRerunPolicy(rerun_maximum_duration).clone();
@@ -273,19 +271,16 @@ StatusOr<CommitResult> Client::Commit(
           .clone();
 
   return Commit(mutator, std::move(default_commit_rerun_policy),
-                std::move(default_commit_backoff_policy), options);
+                std::move(default_commit_backoff_policy));
 }
 
-StatusOr<CommitResult> Client::Commit(Mutations mutations,
-                                      CommitOptions const& options) {
-  return Commit([&mutations](Transaction const&) { return mutations; },
-                options);
+StatusOr<CommitResult> Client::Commit(Mutations mutations) {
+  return Commit([&mutations](Transaction const&) { return mutations; });
 }
 
 StatusOr<CommitResult> Client::Commit(Transaction transaction,
-                                      Mutations mutations,
-                                      CommitOptions const& options) {
-  return conn_->Commit({std::move(transaction), std::move(mutations), options});
+                                      Mutations mutations) {
+  return conn_->Commit({std::move(transaction), std::move(mutations)});
 }
 
 Status Client::Rollback(Transaction transaction) {

google/cloud/spanner/client.cc

@@ -17,7 +17,6 @@
 
 #include "google/cloud/spanner/batch_dml_result.h"
 #include "google/cloud/spanner/client_options.h"
-#include "google/cloud/spanner/commit_options.h"
 #include "google/cloud/spanner/commit_result.h"
 #include "google/cloud/spanner/connection.h"
 #include "google/cloud/spanner/connection_options.h"
@@ -523,7 +522,6 @@ class Client {
    * @param rerun_policy controls for how long (or how many times) the mutator
    *     will be rerun after the transaction aborts.
    * @param backoff_policy controls how long `Commit` waits between reruns.
-   * @param options to apply to the commit.
    *
    * @throw Rethrows any exception thrown by @p `mutator` (after rolling back
    *     the transaction). However, a `RuntimeStatusError` exception is
@@ -536,35 +534,31 @@ class Client {
   StatusOr<CommitResult> Commit(
       std::function<StatusOr<Mutations>(Transaction)> const& mutator,
       std::unique_ptr<TransactionRerunPolicy> rerun_policy,
-      std::unique_ptr<BackoffPolicy> backoff_policy,
-      CommitOptions const& options = {});
+      std::unique_ptr<BackoffPolicy> backoff_policy);
 
   /**
    * Commits a read-write transaction.
    *
    * Same as above, but uses the default rerun and backoff policies.
    *
    * @param mutator the function called to create mutations
-   * @param options to apply to the commit.
    *
    * @par Example
    * @snippet samples.cc commit-with-mutator
    */
   StatusOr<CommitResult> Commit(
-      std::function<StatusOr<Mutations>(Transaction)> const& mutator,
-      CommitOptions const& options = {});
+      std::function<StatusOr<Mutations>(Transaction)> const& mutator);
 
   /**
-   * Commits the @p mutations, using the @p options, atomically in order.
+   * Commits the given @p mutations atomically in order.
    *
    * This function uses the re-run loop described above with the default
    * policies.
    *
    * @par Example
    * @snippet samples.cc commit-with-mutations
    */
-  StatusOr<CommitResult> Commit(Mutations mutations,
-                                CommitOptions const& options = {});
+  StatusOr<CommitResult> Commit(Mutations mutations);
 
   /**
    * Commits a read-write transaction.
@@ -586,13 +580,11 @@ class Client {
    * @param mutations The mutations to be executed when this transaction
    *     commits. All mutations are applied atomically, in the order they appear
    *     in this list.
-   * @param options to apply to the commit.
    *
    * @return A `StatusOr` containing the result of the commit or error status
    *     on failure.
    */
-  StatusOr<CommitResult> Commit(Transaction transaction, Mutations mutations,
-                                CommitOptions const& options = {});
+  StatusOr<CommitResult> Commit(Transaction transaction, Mutations mutations);
 
   /**
    * Rolls back a read-write transaction, releasing any locks it holds.

google/cloud/spanner/client.h

@@ -401,7 +401,7 @@ TEST(ClientTest, CommitMutatorSuccess) {
   auto conn = std::make_shared<MockConnection>();
   Transaction txn = MakeReadWriteTransaction();  // dummy
   Connection::ReadParams actual_read_params{txn, {}, {}, {}, {}, {}};
-  Connection::CommitParams actual_commit_params{txn, {}, {}};
+  Connection::CommitParams actual_commit_params{txn, {}};
 
   auto source = absl::make_unique<MockResultSetSource>();
   auto constexpr kText = R"pb(
@@ -424,7 +424,7 @@ TEST(ClientTest, CommitMutatorSuccess) {
                       Return(ByMove(RowStream(std::move(source))))));
   EXPECT_CALL(*conn, Commit(_))
       .WillOnce(DoAll(SaveArg<0>(&actual_commit_params),
-                      Return(CommitResult{*timestamp, absl::nullopt})));
+                      Return(CommitResult{*timestamp})));
 
   Client client(conn);
   auto mutation = MakeDeleteMutation("table", KeySet::All());
@@ -609,7 +609,7 @@ TEST(ClientTest, CommitMutatorRerunTransientFailures) {
         return Status(StatusCode::kAborted, "Aborted transaction");
       })
       .WillOnce([&timestamp](Connection::CommitParams const&) {
-        return CommitResult{*timestamp, absl::nullopt};
+        return CommitResult{*timestamp};
       });
 
   auto mutator = [](Transaction const&) -> StatusOr<Mutations> {
@@ -681,7 +681,7 @@ TEST(ClientTest, CommitMutations) {
   EXPECT_CALL(*conn, Commit(_))
       .WillOnce([&mutation, &timestamp](Connection::CommitParams const& cp) {
         EXPECT_EQ(cp.mutations, Mutations{mutation});
-        return CommitResult{*timestamp, absl::nullopt};
+        return CommitResult{*timestamp};
       });
 
   Client client(conn);
@@ -782,7 +782,7 @@ TEST(ClientTest, CommitMutatorSessionAffinity) {
             EXPECT_THAT(cp.transaction, HasSession(session_name));
             EXPECT_THAT(cp.transaction, HasBegin());
             SetTransactionId(cp.transaction, "last-transaction-id");
-            return CommitResult{*timestamp, absl::nullopt};
+            return CommitResult{*timestamp};
           });
   // But only after some aborts, the first of which sets the session.
   EXPECT_CALL(*conn, Commit(_))
@@ -821,7 +821,7 @@ TEST(ClientTest, CommitMutatorSessionNotFound) {
   EXPECT_CALL(*conn, Commit(_))
       .WillOnce([&timestamp](Connection::CommitParams const& cp) {
         EXPECT_THAT(cp.transaction, HasSession("session-3"));
-        return CommitResult{*timestamp, absl::nullopt};
+        return CommitResult{*timestamp};
       });
 
   int n = 0;
@@ -851,7 +851,7 @@ TEST(ClientTest, CommitSessionNotFound) {
       })
       .WillOnce([&timestamp](Connection::CommitParams const& cp) {
         EXPECT_THAT(cp.transaction, HasSession("session-2"));
-        return CommitResult{*timestamp, absl::nullopt};
+        return CommitResult{*timestamp};
       });
 
   int n = 0;
@@ -867,28 +867,6 @@ TEST(ClientTest, CommitSessionNotFound) {
   EXPECT_EQ(*timestamp, result->commit_timestamp);
 }
 
-TEST(ClientTest, CommitStats) {
-  auto timestamp =
-      spanner_internal::TimestampFromRFC3339("2020-10-20T02:20:09.123Z");
-  ASSERT_STATUS_OK(timestamp);
-  CommitStats stats{42};
-
-  auto conn = std::make_shared<MockConnection>();
-  EXPECT_CALL(*conn, Commit(_))
-      .WillOnce([&timestamp, &stats](Connection::CommitParams const& cp) {
-        EXPECT_TRUE(cp.options.return_stats());
-        return CommitResult{*timestamp, stats};
-      });
-
-  Client client(conn);
-  auto result =
-      client.Commit(Mutations{}, CommitOptions{}.set_return_stats(true));
-  ASSERT_STATUS_OK(result);
-  EXPECT_EQ(*timestamp, result->commit_timestamp);
-  ASSERT_TRUE(result->commit_stats.has_value());
-  EXPECT_EQ(42, result->commit_stats->mutation_count);
-}
-
 TEST(ClientTest, ProfileQuerySuccess) {
   auto conn = std::make_shared<MockConnection>();
   Client client(conn);

google/cloud/spanner/client_test.cc

@@ -17,30 +17,18 @@
 
 #include "google/cloud/spanner/timestamp.h"
 #include "google/cloud/spanner/version.h"
-#include "absl/types/optional.h"
-#include <cstdint>
 
 namespace google {
 namespace cloud {
 namespace spanner {
 inline namespace SPANNER_CLIENT_NS {
 
-/**
- * Statistics returned for a committed Transaction.
- */
-struct CommitStats {
-  std::int64_t mutation_count;  // total number of mutations
-};
-
 /**
  * The result of committing a Transaction.
  */
 struct CommitResult {
   /// The Cloud Spanner timestamp at which the transaction committed.
   Timestamp commit_timestamp;
-
-  /// Additional statistics about the committed transaction.
-  absl::optional<CommitStats> commit_stats;
 };
 
 }  // namespace SPANNER_CLIENT_NS

google/cloud/spanner/commit_options.h

@@ -16,7 +16,6 @@
 #define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_SPANNER_CONNECTION_H
 
 #include "google/cloud/spanner/batch_dml_result.h"
-#include "google/cloud/spanner/commit_options.h"
 #include "google/cloud/spanner/commit_result.h"
 #include "google/cloud/spanner/connection_options.h"
 #include "google/cloud/spanner/keys.h"
@@ -117,7 +116,6 @@ class Connection {
   struct CommitParams {
     Transaction transaction;
     Mutations mutations;
-    CommitOptions options;
   };
 
   /// Wrap the arguments to `Rollback()`.

google/cloud/spanner/commit_options_test.cc

@@ -23,7 +23,6 @@ google_cloud_cpp_spanner_hdrs = [
     "bytes.h",
     "client.h",
     "client_options.h",
-    "commit_options.h",
     "commit_result.h",
     "connection.h",
     "connection_options.h",