Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13937 )
Change subject: [ksck] Filter tables and tablets in KsckCluster ...................................................................... Patch Set 5: (5 comments) http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck-test.cc File src/kudu/tools/ksck-test.cc: http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck-test.cc@54 PS5, Line 54: #include "kudu/tools/tool_action_common.h" Not sorted correctly. http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck-test.cc@200 PS5, Line 200: virtual Status RetrieveTablesList() override { I don't understand why these changes were needed. Are there any calls to set_table_filters or set_tablet_filters here? http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck.h File src/kudu/tools/ksck.h: http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck.h@452 PS5, Line 452: // Setters for filtering the tables/tablets to be checked. I think having this API on the Ksck class is nicer; that's typically the class that we'd expect people to interact with (and set options on). If you also need something equivalent in KsckCluster for the plumbing, that's fine, but let's keep the Ksck piece and reserve that for user interaction. http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck.cc File src/kudu/tools/ksck.cc: http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck.cc@591 PS5, Line 591: vector<shared_ptr<KsckTablet>> tablets(table->tablets()); Do we actually need to make a copy of this list of tablets? Or could we just operate on table->tablets() directly? http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck_checksum.cc File src/kudu/tools/ksck_checksum.cc: http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck_checksum.cc@502 PS5, Line 502: if (num_tablets == 0 || num_replicas_tmp == 0) { Can you help me understand the motivation for this change? -- To view, visit http://gerrit.cloudera.org:8080/13937 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761 Gerrit-Change-Number: 13937 Gerrit-PatchSet: 5 Gerrit-Owner: Yifan Zhang <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Thu, 01 Aug 2019 23:30:19 +0000 Gerrit-HasComments: Yes
