Bankim Bhavsar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17267 )

Change subject: [master][tool] KUDU-2181 Tool to orchestrate adding a master
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17267/2/src/kudu/tools/tool_action_master.cc
File src/kudu/tools/tool_action_master.cc:

http://gerrit.cloudera.org:8080/#/c/17267/2/src/kudu/tools/tool_action_master.cc@349
PS2, Line 349:   // Get the flags that'll be needed to bring up new master and 
for system catalog copy.
             :   GFlagsMap flags_map = GetFlagsMap();
> What does "passed through" mean here? Is that to say that the flag will have 
> all Master's flags passed to it? If so, could you mention that in the command 
> description?

"Passed through" means in case of following "kudu master add <master-addresses> 
<master-address> --foo_a=val_a --foo_b=val_b" command, flags foo_a and foo_b 
will be passed to the new master being brought up. If those flags are not valid 
then bringing up master will fail. Just like "kudu master run". The flags are 
simply "passed through" without any validation or such. This benefit is not 
unique to approach b. But somehow based on your comment I think you were 
missing that part with approach b.

> Not sure I follow this. If we tell users to supply the arguments as 
> --flag=val, why can't we delimit on space?

Because "--flag_a val_a" is  a valid way to pass a gflag and we'll have to 
restrict users from doing so in this case.
https://gflags.github.io/gflags/#commandline

> One of the downsides with b is that it makes reasoning about the other flags 
> a bit confusing as well, not that they'd actually be different from the 
> Master's flags, but it sits better with me to have the Master flags be a bit 
> more explicit.

That's a fair point.

Imagine a user that has existing masters and simply wants to copy-paste and 
supply master flags. Now the user has to either manually insert delimiters or 
write a script. With approach (b) it's a straight copy-paste. So from user 
perspective it's easy and simple.

We can debate this further in tomorrow's tea time meeting...



--
To view, visit http://gerrit.cloudera.org:8080/17267
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8f99cf2b3f1738c4127c7e7288beab61daf42e7b
Gerrit-Change-Number: 17267
Gerrit-PatchSet: 2
Gerrit-Owner: Bankim Bhavsar <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Bankim Bhavsar <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 05 Apr 2021 20:17:40 +0000
Gerrit-HasComments: Yes

Reply via email to