Will Berkeley 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 4: (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(): Done 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 c These particular errors come back as RemoteError. http://gerrit.cloudera.org:8080/#/c/10300/3/src/kudu/tools/ksck.cc@399 PS3, Line 399: re-run ksck wit > nit: maybe just 're-run ksck ...' Done 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: I consider WRONG_SERVER_UUID to be worse because it means there's a tablet server at address X that other servers did not expect to be there, which indicates some kind of disaster like the node was wiped out and restarted, or bad misconfiguration. Is it worse if you go to your friend's house and knock on the door and they aren't home or if you knock on the door and some other dude answers? :) 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 It's not UNKNOWN, it's UNAUTHORIZED. I agree ServerHealth has already grown beyond health. ServerStatus is kind of better except it wouldn't actually be of type Status. We don't list all servers and their errors at the end of the ksck output. They are surfaced earlier, and there we do include both the status message and the ServerHealth. -- 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: 4 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:34:09 +0000 Gerrit-HasComments: Yes
