commit 3faeb3d9c6c87e4d6815e06945f331a4a14402ba
author Andrew Wong <awong@cloudera.com> Thu Feb 25 16:18:52 2021 -0800
committer Andrew Wong <awong@cloudera.com> Fri Feb 26 22:07:40 2021 +0000
tree 7d1ee2ca80a7a8cf562224774b64079ae3417ef9
parent 70e0998fb1003763e596595c1b3ebb57c3d32bf3

KUDU-2612: don't return NOT_FOUND when BEGIN_TXN has not yet run

In testing an in-flight patch[1], it was found that our current handling of
errors when participant registration hasn't completed can lead to
incorrect behavior: the path in which we handle NOT_FOUND errors while
committing expects that such errors only occur if the tablet has been
deleted, and we currently proceed with the commit. The incorrect
assumption here was that NOT_FOUND errors are only produced when the
tablet has been deleted, which is not the case.

This behavior will be amended in an upcoming patch[2], and we'll
actually abort the transaction on NOT_FOUND errors. Until then, this
patch adjust the behavior to return ILLEGAL_STATE instead of NOT_FOUND,
so at least the error type is consistent with the rest of the
participant op lifecycle errors.

[1] https://gerrit.cloudera.org/c/17037/
[2] https://gerrit.cloudera.org/c/17022

Change-Id: I8fa8caba384ee30536114a3e6466ad90b6d8e45d
Reviewed-on: http://gerrit.cloudera.org:8080/17127
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <aserbin@cloudera.com>
Reviewed-by: Hao Hao <hao.hao@cloudera.com>

src/kudu/tablet/txn_participant-test.cc

diff --git a/src/kudu/tablet/txn_participant-test.cc b/src/kudu/tablet/txn_participant-test.cc
index 7cdbc39..ca8af95 100644
--- a/src/kudu/tablet/txn_participant-test.cc
+++ b/src/kudu/tablet/txn_participant-test.cc
@@ -256,7 +256,7 @@
       ASSERT_TRUE(resp.has_error());
       ASSERT_TRUE(resp.error().has_status());
       ASSERT_EQ(TabletServerErrorPB::UNKNOWN_ERROR, resp.error().code());
-      ASSERT_EQ(AppStatusPB::NOT_FOUND, resp.error().status().code());
+      ASSERT_EQ(AppStatusPB::ILLEGAL_STATE, resp.error().status().code());
       ASSERT_FALSE(resp.has_timestamp());
     }
   };

src/kudu/tablet/txn_participant.h

diff --git a/src/kudu/tablet/txn_participant.h b/src/kudu/tablet/txn_participant.h
index d276840..c497f04 100644
--- a/src/kudu/tablet/txn_participant.h
+++ b/src/kudu/tablet/txn_participant.h
@@ -140,7 +140,7 @@
       Timestamp timestamp;
       TxnState meta_state;
       if (!tablet_metadata_->HasTxnMetadata(txn_id_, &meta_state, &timestamp)) {
-        return Status::NotFound("Transaction hasn't been successfully started");
+        return Status::IllegalState("Transaction hasn't been successfully started");
       }
       if (PREDICT_FALSE(meta_state != kCommitted && meta_state != kCommitInProgress)) {
         *code = tserver::TabletServerErrorPB::TXN_ILLEGAL_STATE;
@@ -293,7 +293,7 @@
                                 txn_id_));
       }
       // TODO(awong): add another code for this?
-      return Status::NotFound("Transaction hasn't been successfully started");
+      return Status::IllegalState("Transaction hasn't been successfully started");
     }
     return Status::OK();
   }