Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10151 )
Change subject: [tools] ksck improvements [6/n]: Refactor result handling ...................................................................... Patch Set 11: (5 comments) http://gerrit.cloudera.org:8080/#/c/10151/11//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10151/11//COMMIT_MSG@20 PS11, Line 20: - Add the changes in https://gerrit.cloudera.org/#/c/10054/, which were : about to be committed but would've needed to be redone a bit to fit : with this refactor. Just checking, is this the only user-facing change, other than some minor syntax? http://gerrit.cloudera.org:8080/#/c/10151/9/src/kudu/tools/ksck-test.cc File src/kudu/tools/ksck-test.cc: http://gerrit.cloudera.org:8080/#/c/10151/9/src/kudu/tools/ksck-test.cc@674 PS9, Line 674: > Done Ah! Seems you missed this one and others similar. http://gerrit.cloudera.org:8080/#/c/10151/11/src/kudu/tools/ksck.cc File src/kudu/tools/ksck.cc: http://gerrit.cloudera.org:8080/#/c/10151/11/src/kudu/tools/ksck.cc@297 PS11, Line 297: s.IsRemoteError() ? : summary.health = KsckServerHealth::WRONG_SERVER_UUID : Not your fault, but I don't think this is always the case. I just tried running a relatively recent ksck on a cluster and got the following remote errors for all tablet servers: WARNING: Errors gathering consensus info for Tablet Server 70f7ee61ead54b1885d819f354eb3405 (vc1316.halxg.cloudera.com:7050): Remote error: could not fetch all consensus info: Not authorized: unauthorized access to method: GetConsensusState And so all of the tservers incorrectly reported WRONG_SERVER_UUID. http://gerrit.cloudera.org:8080/#/c/10151/11/src/kudu/tools/ksck.cc@340 PS11, Line 340: PUSH_PREPEND_NOT_OK(FetchInfoFromTabletServers(), results_.error_messages, : "error fetching info from tablet servers"); : Why isn't this a prereq for the below checks? http://gerrit.cloudera.org:8080/#/c/10151/11/src/kudu/tools/tool_action_cluster.cc File src/kudu/tools/tool_action_cluster.cc: http://gerrit.cloudera.org:8080/#/c/10151/11/src/kudu/tools/tool_action_cluster.cc@a146 PS11, Line 146: Now that this patch is keeping `--consensus`, this probably isn't needed? -- To view, visit http://gerrit.cloudera.org:8080/10151 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id8de619996b6cd753e6a9c01b1b60810a873e609 Gerrit-Change-Number: 10151 Gerrit-PatchSet: 11 Gerrit-Owner: Will Berkeley <wdberke...@gmail.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-Comment-Date: Fri, 27 Apr 2018 18:47:25 +0000 Gerrit-HasComments: Yes
