Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/8804 )
Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode ...................................................................... Patch Set 16: (6 comments) http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_server-test.cc File src/kudu/tserver/tablet_server-test.cc: http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_server-test.cc@a1726 PS16, Line 1726: What happened to this test? Maybe I'm missing something, but it seems this test should stay relevant regardless of the new scan mode. http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.h File src/kudu/tserver/tablet_service.h: http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.h@157 PS16, Line 157: TimeStamp nit: in the code around, we have Timestamp as a type; maybe, name it consistently with that so it's ValidateTimestamp(...) ? http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.h@161 PS16, Line 161: PickAndVerifyTimeStamp nit: ditto, PickAndVerifyTimestamp(...) ? http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.h@162 PS16, Line 162: tablet::TabletReplica* Could it be const tablet::TabletReplica& ? Or, maybe, Tablet* ? http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc File src/kudu/tserver/tablet_service.cc: http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@1757 PS16, Line 1757: case READ_YOUR_WRITES: nit: per coding standard, add // fallthrough http://gerrit.cloudera.org:8080/#/c/8804/16/src/kudu/tserver/tablet_service.cc@2042 PS16, Line 2042: case READ_AT_SNAPSHOT: nit: per coding standard, add // fallthrough -- To view, visit http://gerrit.cloudera.org:8080/8804 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48 Gerrit-Change-Number: 8804 Gerrit-PatchSet: 16 Gerrit-Owner: Hao Hao <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Sat, 17 Feb 2018 01:44:55 +0000 Gerrit-HasComments: Yes
