Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10300 )
Change subject: [tools] ksck imp. [8/n]: Easy-to-see errors when running w/o admin ...................................................................... Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/10300/3/src/kudu/tools/ksck-test.cc File src/kudu/tools/ksck-test.cc: http://gerrit.cloudera.org:8080/#/c/10300/3/src/kudu/tools/ksck-test.cc@769 PS3, Line 769: ASSERT_TRUE(s.IsNotAuthorized()); nit: maybe it's worth adding s.ToString(): ASSERT_TRUE(...) << s.ToString(); http://gerrit.cloudera.org:8080/#/c/10300/3/src/kudu/tools/ksck.cc File src/kudu/tools/ksck.cc: http://gerrit.cloudera.org:8080/#/c/10300/3/src/kudu/tools/ksck.cc@146 PS3, Line 146: s.IsRemoteError I thought some of the errors are returned back as NotAuthorized() in that case, no? http://gerrit.cloudera.org:8080/#/c/10300/3/src/kudu/tools/ksck.cc@399 PS3, Line 399: please run ksck nit: maybe just 're-run ksck ...' http://gerrit.cloudera.org:8080/#/c/10300/3/src/kudu/tools/ksck_results.cc File src/kudu/tools/ksck_results.cc: http://gerrit.cloudera.org:8080/#/c/10300/3/src/kudu/tools/ksck_results.cc@171 PS3, Line 171: case KsckServerHealth::UNAUTHORIZED: : return 1; : case KsckServerHealth::UNAVAILABLE: : return 2; : case KsckServerHealth::WRONG_SERVER_UUID: I would expect that to be in different order: UNAUTHORIZED WRONG_SERVER_UUID UNAVAILABLE Not sure it makes a big difference, but unavailable/dead server sounds like having worse health than server which is alive but has wrong UUID. But that started with the previous revision, though. http://gerrit.cloudera.org:8080/#/c/10300/3/src/kudu/tools/tool.proto File src/kudu/tools/tool.proto: http://gerrit.cloudera.org:8080/#/c/10300/3/src/kudu/tools/tool.proto@224 PS3, Line 224: UNAUTHORIZED = 3; It becoming less and less ServerHealth-related, but I cannot suggest a good alternative, though. Maybe, just use UNKNOWN? Does it help to have that status in addition to the error summary in the end of the ksck output? -- To view, visit http://gerrit.cloudera.org:8080/10300 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I644957103efefceeba4b82f4557376a603507423 Gerrit-Change-Number: 10300 Gerrit-PatchSet: 3 Gerrit-Owner: Will Berkeley <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Fri, 25 May 2018 19:06:22 +0000 Gerrit-HasComments: Yes
