Alexey Serbin 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:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/17017/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17017/5//COMMIT_MSG@10
PS5, Line 10: when TxnStatusManager::AbortTransaction()
nit: when TxnStatusManager::AbortTransaction() is called

?


http://gerrit.cloudera.org:8080/#/c/17017/5/src/kudu/integration-tests/txn_commit-itest.cc
File src/kudu/integration-tests/txn_commit-itest.cc:

http://gerrit.cloudera.org:8080/#/c/17017/5/src/kudu/integration-tests/txn_commit-itest.cc@262
PS5, Line 262: (const auto& txn_id_and_count : aborts_per_txn) {
nit: if you interested in syntactic sugar by any chance, with C++17 it's 
possible to write this as

  for (const auto& [txn_id, count] : aborts_per_txn) {
    ...
  }


http://gerrit.cloudera.org:8080/#/c/17017/5/src/kudu/integration-tests/txn_commit-itest.cc@330
PS5, Line 330:  ASSERT_OK(txn->IsCommitComplete(&is_complete, 
&completion_status));
So, this now reports IsAborted() for both TXN_ABORTED and 
TXN_ABORT_IN_PROGRESS, right?


http://gerrit.cloudera.org:8080/#/c/17017/5/src/kudu/integration-tests/txn_commit-itest.cc@332
PS5, Line 332: is_complete
What should we report on a status of a transaction in ABORT_IN_PROGRESS state?  
Should IsCommitComplete():
  * return 'false' for 'is_complete' if txn is in TXN_ABORT_IN_PROGRESS
  * return 'true' for 'is_complete' if txn is in ABORTED

?


http://gerrit.cloudera.org:8080/#/c/17017/5/src/kudu/integration-tests/txn_commit-itest.cc@784
PS5, Line 784: commit
nit: finalize commit and abort

?



--
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: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 08 Feb 2021 22:55:28 +0000
Gerrit-HasComments: Yes

Reply via email to