Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14456 )
Change subject: [clock] handle corner case of NowWithError() in Update() ...................................................................... Patch Set 1: (2 comments) Could we test this, perhaps using the mock time source? http://gerrit.cloudera.org:8080/#/c/14456/1/src/kudu/clock/hybrid_clock.cc File src/kudu/clock/hybrid_clock.cc: http://gerrit.cloudera.org:8080/#/c/14456/1/src/kudu/clock/hybrid_clock.cc@218 PS1, Line 218: void HybridClock::NowWithError(Timestamp* timestamp, uint64_t* max_error_usec) { On a related note, I'm pretty sure NowWithError must be called with lock_ held. So we should rename it to Unlocked, add an AssertLocked, and probably make it private too. http://gerrit.cloudera.org:8080/#/c/14456/1/src/kudu/clock/hybrid_clock.cc@258 PS1, Line 258: DCHECK_GE(next_timestamp_ >> kBitsToShift, now_usec - error_usec); Maybe pull these out of the DCHECK and into locals so they can be shared with L259? -- To view, visit http://gerrit.cloudera.org:8080/14456 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I701eed492442bc6360d4c8f02f1bfc3ee96a3b90 Gerrit-Change-Number: 14456 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 15 Oct 2019 20:22:27 +0000 Gerrit-HasComments: Yes
