Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10151 )
Change subject: [tools] ksck improvements [6/n]: Refactor result handling ...................................................................... Patch Set 9: (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: 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 s Yes. 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: ASSERT_STR_CONTAINS > 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: : } > Not your fault, but I don't think this is always the case. I just tried run :/ I hacked in a more precise but more brittle check. I'm not sure we can so better without returning more information from the fetching, which would be some work to integrate. http://gerrit.cloudera.org:8080/#/c/10151/11/src/kudu/tools/ksck.cc@340 PS11, Line 340: PUSH_PREPEND_NOT_OK(ChecksumData(ChecksumOptions()), : results_.error_messages, "checksum scan error"); : > Why isn't this a prereq for the below checks? Because tablet servers may fail to give us info and we can still do checks. If we fail to get info from the leader master we are donezo. 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? Done -- 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: 9 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: Sat, 28 Apr 2018 00:58:47 +0000 Gerrit-HasComments: Yes
