Todd Lipcon has posted comments on this change. Change subject: Simplify OpId/Timestamp assignment and make it atomic ......................................................................
Patch Set 8: (2 comments) need to look in more detail at the changes in consensus/ but some quicker questions about the MvccManager design http://gerrit.cloudera.org:8080/#/c/7221/8/src/kudu/tablet/transactions/transaction_driver.cc File src/kudu/tablet/transactions/transaction_driver.cc: Line 267: prepare_state_ = PREPARED; it's sketching me out that the state transition to PREPARED happens under this lock acquisition for followers but not until down below for leaders. Why can't we keep it as it was? Separate note: I think the big block comment in the header needs to be tweaked a bit. PS8, Line 295: // Register a dummy transaction with mvcc manager that we'll abort later. instead of using a dummy transaction, would it be possible to register it early with the current clock time, as you've done here, and add a new API like MvccManager::PushTransactionTimestamp(scoped_txn, new_timestamp) which changes the timestamp of the existing entry? This would avoid an allocation at least and can include an assertion that you can only PushTransactionTimestamp to a later timestamp -- To view, visit http://gerrit.cloudera.org:8080/7221 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2bedcc5c3708d06f48131f34ea14c835867b4800 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
