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
commit 3faeb3d9c6c87e4d6815e06945f331a4a14402ba Author: Andrew Wong <[email protected]> AuthorDate: Thu Feb 25 16:18:52 2021 -0800 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 <[email protected]> Reviewed-by: Hao Hao <[email protected]> --- src/kudu/tablet/txn_participant-test.cc | 2 +- src/kudu/tablet/txn_participant.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) 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 @@ TEST_F(TxnParticipantTest, TestTransactionNotFound) { 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()); } }; 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 @@ class Txn : public RefCountedThreadSafe<Txn> { Timestamp timestamp; TxnState meta_state; if (!tablet_metadata_->HasTxnMetadata(txn_id_, &meta_state, ×tamp)) { - 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 @@ class Txn : public RefCountedThreadSafe<Txn> { 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(); }
