[kudu] branch master updated: KUDU-2612: make BeginCommit return OK if already committed

[aw...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

awong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new b73f759  KUDU-2612: make BeginCommit return OK if already committed
b73f759 is described below

commit b73f75927f703eb7a5fbaaed525fd9c6b8c140f1
Author: Andrew Wong <aw...@cloudera.com>
AuthorDate: Tue Feb 16 15:42:58 2021 -0800

    KUDU-2612: make BeginCommit return OK if already committed
    
    This patch makes repeat calls to BeginCommit by the TxnStatusManager
    return OK rather than an IllegalState error. This has previously
    surfaced in flakiness in some tests, wherein repeated calls to
    TxnManager::Commit() yields an IllegalState error since the commit has
    already completed.
    
    This also reverts 67018be8ba27480b050c11504df8a732f6a52daf that
    addressed this by increasing commit latency.
    
    Change-Id: I420ba1c5460d59103984ea9a1f16177b82b8d0e1
    Reviewed-on: http://gerrit.cloudera.org:8080/17073
    Reviewed-by: Alexey Serbin <as...@cloudera.com>
    Tested-by: Andrew Wong <aw...@cloudera.com>
---
 src/kudu/integration-tests/txn_commit-itest.cc   | 1 -
 src/kudu/transactions/txn_status_manager-test.cc | 8 ++------
 src/kudu/transactions/txn_status_manager.cc      | 3 ++-
 3 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/src/kudu/integration-tests/txn_commit-itest.cc b/src/kudu/integration-tests/txn_commit-itest.cc
index c43fc1d..17f2078 100644
--- a/src/kudu/integration-tests/txn_commit-itest.cc
+++ b/src/kudu/integration-tests/txn_commit-itest.cc
@@ -682,7 +682,6 @@ TEST_F(TxnCommitITest, TestConcurrentCommitCalls) {
 // Test that committing the same transaction concurrently doesn't lead to any
 // issues.
 TEST_F(TxnCommitITest, TestConcurrentRepeatedCommitCalls) {
-  FLAGS_txn_status_manager_inject_latency_finalize_commit_ms = 1000;
   shared_ptr<KuduTransaction> txn;
   shared_ptr<KuduSession> txn_session;
   ASSERT_OK(BeginTransaction(participant_ids_, &txn, &txn_session));
diff --git a/src/kudu/transactions/txn_status_manager-test.cc b/src/kudu/transactions/txn_status_manager-test.cc
index 35fa2da..61f38c6 100644
--- a/src/kudu/transactions/txn_status_manager-test.cc
+++ b/src/kudu/transactions/txn_status_manager-test.cc
@@ -806,14 +806,10 @@ TEST_F(TxnStatusManagerTest, TestUpdateTransactionState) {
   s = txn_manager_->AbortTransaction(kTxnId2, kOwner, &ts_error);
   ASSERT_TRUE(s.IsIllegalState()) << s.ToString();
 
-  // Redundant finalize calls are also benign.
+  // Redundant begin commit or finalize calls are also benign.
+  ASSERT_OK(txn_manager_->BeginCommitTransaction(kTxnId2, kOwner, &ts_error));
   ASSERT_OK(txn_manager_->FinalizeCommitTransaction(
       kTxnId2, Timestamp::kInitialTimestamp, &ts_error));
-
-  // Calls to begin committing should return an error if we've already
-  // finalized the commit.
-  s = txn_manager_->BeginCommitTransaction(kTxnId2, kOwner, &ts_error);
-  ASSERT_TRUE(s.IsIllegalState()) << s.ToString();
 }
 
 // Test that we can only add participants to a transaction when it's in an
diff --git a/src/kudu/transactions/txn_status_manager.cc b/src/kudu/transactions/txn_status_manager.cc
index 33f2df8..e6ec4da 100644
--- a/src/kudu/transactions/txn_status_manager.cc
+++ b/src/kudu/transactions/txn_status_manager.cc
@@ -923,7 +923,8 @@ Status TxnStatusManager::BeginCommitTransaction(int64_t txn_id, const string& us
   TransactionEntryLock txn_lock(txn.get(), LockMode::WRITE);
   const auto& pb = txn_lock.data().pb;
   const auto& state = pb.state();
-  if (state == TxnStatePB::COMMIT_IN_PROGRESS) {
+  if (state == TxnStatePB::COMMIT_IN_PROGRESS ||
+      state == TxnStatePB::COMMITTED) {
     return Status::OK();
   }
   if (PREDICT_FALSE(state != TxnStatePB::OPEN)) {