Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11422 )
Change subject: [location_awareness] Add location info in ksck report ...................................................................... Patch Set 12: (3 comments) The location isn't getting piped through to the JSON output. You'll also need to . Add the location to the KsckServerHealthSummaryPB in tool.proto. . Set the location field in the proto in KsckServerHealthSummaryToPb in ksck_results.cc . Enhance CheckJsonVsServerHealthSummaries in ksck-test.cc so it checks the locations set in the json vs the locations set in the summaries. http://gerrit.cloudera.org:8080/#/c/11422/12//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11422/12//COMMIT_MSG@24 PS12, Line 24: Location > As for the order of fields, I think keeping the 'Status' before the 'Locati +1 http://gerrit.cloudera.org:8080/#/c/11422/12//COMMIT_MSG@31 PS12, Line 31: Location | Count > nit: maybe, add a title for this table -- 'Tablet Server Location Summary' +1 http://gerrit.cloudera.org:8080/#/c/11422/12/src/kudu/tools/ksck_remote.h File src/kudu/tools/ksck_remote.h: http://gerrit.cloudera.org:8080/#/c/11422/12/src/kudu/tools/ksck_remote.h@95 PS12, Line 95: const > nit: what does it buy having const for the HostPort parameter passed by val It should either by a const reference or non-const and passed-by-value with std::move if HostPort has a move constructor. -- To view, visit http://gerrit.cloudera.org:8080/11422 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ideff2dd4975c99a1135002624debbbb2620fb95c Gerrit-Change-Number: 11422 Gerrit-PatchSet: 12 Gerrit-Owner: Fengling Wang <fw...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Fengling Wang <fw...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-Comment-Date: Mon, 29 Oct 2018 20:18:15 +0000 Gerrit-HasComments: Yes
