Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18124 )
Change subject: KUDU-2915: add tool to unregister a tablet server ...................................................................... Patch Set 8: Code-Review+1 (2 comments) Overall LGTM, thanks! Just a tiny nit with error message formatting. http://gerrit.cloudera.org:8080/#/c/18124/5/src/kudu/master/master_service.cc File src/kudu/master/master_service.cc: http://gerrit.cloudera.org:8080/#/c/18124/5/src/kudu/master/master_service.cc@351 PS5, Line 351: > This is in the condition where the status is not 'NotFound' so it should be Ah, that makes sense: it seems I misunderstood this code. http://gerrit.cloudera.org:8080/#/c/18124/8/src/kudu/tools/tool_action_tserver.cc File src/kudu/tools/tool_action_tserver.cc: http://gerrit.cloudera.org:8080/#/c/18124/8/src/kudu/tools/tool_action_tserver.cc@332 PS8, Line 332: "Unable to unregister the tserver from master $0, status: $1" nit: maybe add spacing between consecutive messages, so the result error message would be more readable? Adding a space in the beginning of each message should be good enough, I guess. -- To view, visit http://gerrit.cloudera.org:8080/18124 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If1f5c2979a8d14428f4bcc8e850c57ce228c793a Gerrit-Change-Number: 18124 Gerrit-PatchSet: 8 Gerrit-Owner: Yifan Zhang <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Comment-Date: Fri, 21 Jan 2022 22:03:14 +0000 Gerrit-HasComments: Yes
