Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9641 )
Change subject: Add a way to pin clean time advancement ...................................................................... Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/9641/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9641/3//COMMIT_MSG@15 PS3, Line 15: add nit: adds http://gerrit.cloudera.org:8080/#/c/9641/3/src/kudu/tablet/mvcc.h File src/kudu/tablet/mvcc.h: http://gerrit.cloudera.org:8080/#/c/9641/3/src/kudu/tablet/mvcc.h@370 PS3, Line 370: PinCleanTimeAdvanceTo nit: how about PinCleanTimeAdvancement() and UnpinCleanTimeAdvancement(). http://gerrit.cloudera.org:8080/#/c/9641/3/src/kudu/tablet/mvcc.cc File src/kudu/tablet/mvcc.cc: http://gerrit.cloudera.org:8080/#/c/9641/3/src/kudu/tablet/mvcc.cc@217 PS3, Line 217: // There should only be one pin at a time. Hrmm.. Is this because the prepare thread is single-threaded, and so we are guaranteed to only assign (and thus may only pin) a single timestamp at a time? http://gerrit.cloudera.org:8080/#/c/9641/3/src/kudu/tablet/mvcc.cc@229 PS3, Line 229: MvccManager::AdjustCleanTime() { While you're in the area, would you mind renaming this to AdjustCleanTimeUnlocked() and DCHECKing on lock_? -- To view, visit http://gerrit.cloudera.org:8080/9641 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If863d28016aee98672d94ec5dc8a8630e1cbf5c8 Gerrit-Change-Number: 9641 Gerrit-PatchSet: 3 Gerrit-Owner: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Tue, 20 Mar 2018 21:24:38 +0000 Gerrit-HasComments: Yes
