Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17219 )
Change subject: [master] Check master consensus and unify success message ...................................................................... Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/17219/1/src/kudu/tools/tool_action_master.cc File src/kudu/tools/tool_action_master.cc: http://gerrit.cloudera.org:8080/#/c/17219/1/src/kudu/tools/tool_action_master.cc@242 PS1, Line 242: } : Would it make sense to log a warning in the error case? Would that be too verbose? Would that even be useful? http://gerrit.cloudera.org:8080/#/c/17219/1/src/kudu/tools/tool_action_master.cc@251 PS1, Line 251: LOG(INFO) << Substitute("Master $0 could not be caught up from WAL. Please follow the next steps " : "which includes system catalog tablet copy, updating master addresses " : "etc. from the Kudu administration documentation on " : "\"Migrating to Multiple Kudu Masters\".", I made a similar point in an earlier review, but having the script grep for messages has a bit of code smell, especially if we don't ship the script as a part of the binary. I'd much prefer putting the scripted logic into this tool, or perhaps leveraging different error codes to signal that we're ready to make the copy, since it'd define a more concrete API with which to determine next orchestration steps. -- To view, visit http://gerrit.cloudera.org:8080/17219 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1eb49e92fb001b44f0d53ff85f81c365bff24214 Gerrit-Change-Number: 17219 Gerrit-PatchSet: 1 Gerrit-Owner: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 29 Mar 2021 21:18:43 +0000 Gerrit-HasComments: Yes
