Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17017 )

Change subject: KUDU-2612: add background task to abort transaction participants
......................................................................


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/17017/5/src/kudu/client/transaction-internal.cc
File src/kudu/client/transaction-internal.cc:

http://gerrit.cloudera.org:8080/#/c/17017/5/src/kudu/client/transaction-internal.cc@309
PS5, Line 309: case TxnStatePB::ABORT_IN_PROGRESS:
             :     case TxnStatePB::ABORTED:
             :       *is_complete = true;
             :       *completion_status = Status::Aborted("transaction has been 
aborted");
I understand we consider ABORT_IN_PROGRESS to be eventually aborted no matter 
what. But do you think it still worth returning different error msg to 
differentiate txn that is actually aborted v.s. still aborting?


http://gerrit.cloudera.org:8080/#/c/17017/5/src/kudu/transactions/txn_status_manager.cc
File src/kudu/transactions/txn_status_manager.cc:

http://gerrit.cloudera.org:8080/#/c/17017/5/src/kudu/transactions/txn_status_manager.cc@331
PS5, Line 331: write the finalized
             :     // commit timestamp to the tablet.
nit: update to write the abort record?


http://gerrit.cloudera.org:8080/#/c/17017/5/src/kudu/transactions/txn_status_manager.cc@982
PS5, Line 982: mutable_data->pb.set_commit_timestamp(commit_timestamp.value());
             :   RETURN_NOT_OK(status_tablet_.UpdateTransaction(
             :       txn_id, mutable_data->pb, ts_error));
             :
             :   {
             :     std::lock_guard<simple_spinlock> l(lock_);
             :     commits_in_flight_.erase(txn_id);
             :   }
             :
Just curious, it seems this should belong to the previous patch which 
introduced background task to commit transactions?



--
To view, visit http://gerrit.cloudera.org:8080/17017
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I484c315c6f7331c5ec12cb06370fbaae9c7c343e
Gerrit-Change-Number: 17017
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Hao Hao <hao....@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 09 Feb 2021 06:50:47 +0000
Gerrit-HasComments: Yes

Reply via email to