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 8: (3 comments) http://gerrit.cloudera.org:8080/#/c/11427/5/src/kudu/consensus/time_manager.cc File src/kudu/consensus/time_manager.cc: http://gerrit.cloudera.org:8080/#/c/11427/5/src/kudu/consensus/time_manager.cc@81 PS5, Line 81: } > Is this needed? Yeah, the clock's initial value may be we may end up assigning a timestamp of 1 to the first timestamp, and if that happens to be an election no-op, clients will refuse to scan. That said, updating state in the constructor is code smell, so I'll move this out of here. http://gerrit.cloudera.org:8080/#/c/11427/5/src/kudu/tablet/tablet_replica.cc File src/kudu/tablet/tablet_replica.cc: http://gerrit.cloudera.org:8080/#/c/11427/5/src/kudu/tablet/tablet_replica.cc@594 PS5, Line 594: icate_msg->op > Do we want to adjust the same time while it's bootstrapping? Won't that hap You're right that during bootstrapping, we update the safe time when we find replicate/commit pairs. That wouldn't be the case here. We'll get here during bootstrap while replaying orphaned replicates, and we can also get here if consensus is actually started and responding to requests while the tablet is still in bootstrapping. I.e. there doesn't seem to be a "double updating" http://gerrit.cloudera.org:8080/#/c/11427/5/src/kudu/tablet/tablet_replica.cc@605 PS5, Line 605: } > Based on the implementation, we don't have to, but I wonder if we should ma Added a comment, and made a copy. Right, when it comes to these back-references, the hierarchy of pointers gets messed up and it's probably better to be a bit more conservative. -- 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: 8 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, 20 Sep 2018 18:16:40 +0000 Gerrit-HasComments: Yes
