Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11427 )
Change subject: KUDU-2463 pt 2: bump MVCC safe time on Raft no-op ...................................................................... Patch Set 13: (13 comments) http://gerrit.cloudera.org:8080/#/c/11427/12/src/kudu/consensus/raft_consensus.h File src/kudu/consensus/raft_consensus.h: http://gerrit.cloudera.org:8080/#/c/11427/12/src/kudu/consensus/raft_consensus.h@929 PS12, Line 929: // Consensus-only rounds complete when non-transaction ops finish > this method should be documented in the interface, either in the block comm Done here http://gerrit.cloudera.org:8080/#/c/11427/12/src/kudu/tablet/mvcc.cc File src/kudu/tablet/mvcc.cc: http://gerrit.cloudera.org:8080/#/c/11427/12/src/kudu/tablet/mvcc.cc@190 PS12, Line 190: Getting here means that we are about to apply a transaction out of : // order > makes sense; did you observe this? Yeah, I've seen this here and there when there is a lot of injected delays to the consensus queue (eg exactly_once_writes-itest and timestamp_advancement-itest) http://gerrit.cloudera.org:8080/#/c/11427/12/src/kudu/tablet/tablet_replica.cc File src/kudu/tablet/tablet_replica.cc: http://gerrit.cloudera.org:8080/#/c/11427/12/src/kudu/tablet/tablet_replica.cc@646 PS12, Line 646: no-op > no-op used to assert term leadership Done http://gerrit.cloudera.org:8080/#/c/11427/12/src/kudu/tablet/tablet_replica.cc@647 PS12, Line 647: estamps > in the same and later Done http://gerrit.cloudera.org:8080/#/c/11427/12/src/kudu/tablet/tablet_replica.cc@648 PS12, Line 648: such, we > using the timestamp of such no-ops Done http://gerrit.cloudera.org:8080/#/c/11427/12/src/kudu/tablet/tablet_replica.cc@649 PS12, Line 649: n > them Done http://gerrit.cloudera.org:8080/#/c/11427/12/src/kudu/tablet/tablet_replica.cc@651 PS12, Line 651: > s/such/MVCC safe time/ Done http://gerrit.cloudera.org:8080/#/c/11427/12/src/kudu/tablet/tablet_replica.cc@653 PS12, Line 653: e time > s/started/prepared/ Done http://gerrit.cloudera.org:8080/#/c/11427/12/src/kudu/tablet/tablet_replica.cc@653 PS12, Line 653: the > s/submit/run/ Done http://gerrit.cloudera.org:8080/#/c/11427/12/src/kudu/tablet/tablet_replica.cc@654 PS12, Line 654: no-op of term N after all tran > the safe time adjustment function on the Done http://gerrit.cloudera.org:8080/#/c/11427/12/src/kudu/tablet/tablet_replica.cc@654 PS12, Line 654: N-1 have been prepared, w > which is the same mechanism we use to serialize transactions Done http://gerrit.cloudera.org:8080/#/c/11427/12/src/kudu/tablet/tablet_replica.cc@656 PS12, Line 656: echanism we use to serialize transactions. : // : // If the 'timestamp_in_opid_order' flag is unset, the no-op is assumed to be : // the Raft leadership no-op from a version of Kudu that only supported creating : // a no-op to assert a new leadership term, in which case it would be in order. : if (op_type == consensus::NO_OP && : (!replicate_msg->noop_request().has_timestamp_in_opid_order( > can we just combine these if statements? i.e. Done http://gerrit.cloudera.org:8080/#/c/11427/12/src/kudu/tablet/tablet_replica.cc@664 PS12, Line 664: HECK(rep > I just traced this to make sure, but we may want to add comment with someth Done -- To view, visit http://gerrit.cloudera.org:8080/11427 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icbf812e2cbeeee7c322fd980245cfe40c886a15a Gerrit-Change-Number: 11427 Gerrit-PatchSet: 13 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Thu, 11 Oct 2018 21:22:45 +0000 Gerrit-HasComments: Yes
