You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org> on 2021/03/23 21:13:07 UTC
[kudu-CR] [master] Check master consensus and unify success message
Bankim Bhavsar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17219
Change subject: [master] Check master consensus and unify success message
......................................................................
[master] Check master consensus and unify success message
Motivation of this change is to have a common success message
whether system catalog copy is needed or not and use
a common message format which is easy to grep from a script.
Checking master consensus from the CLI tool is a better
way to check whether new master has successfully caught
up from WAL.
Change-Id: I1eb49e92fb001b44f0d53ff85f81c365bff24214
---
M src/kudu/master/dynamic_multi_master-test.cc
M src/kudu/tools/tool_action_master.cc
2 files changed, 65 insertions(+), 25 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/17219/1
--
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: newchange
Gerrit-Change-Id: I1eb49e92fb001b44f0d53ff85f81c365bff24214
Gerrit-Change-Number: 17219
Gerrit-PatchSet: 1
Gerrit-Owner: Bankim Bhavsar <ba...@cloudera.com>
[kudu-CR] [master] Check master consensus and unify success message
Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has abandoned this change. ( http://gerrit.cloudera.org:8080/17219 )
Change subject: [master] Check master consensus and unify success message
......................................................................
Abandoned
Moving the orchestration logic to the CLI tool would overhaul this piece of code.
--
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: abandon
Gerrit-Change-Id: I1eb49e92fb001b44f0d53ff85f81c365bff24214
Gerrit-Change-Number: 17219
Gerrit-PatchSet: 1
Gerrit-Owner: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [master] Check master consensus and unify success message
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
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 <ba...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 29 Mar 2021 21:18:43 +0000
Gerrit-HasComments: Yes