Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17220 )
Change subject: [master] Script to automate adding a master ...................................................................... Patch Set 3: > > Patch Set 2: > > > > > Patch Set 1: > > > > > > > Patch Set 1: > > > > > > > > > Patch Set 1: Verified-1 > > > > > > > > > > Build Failed > > > > > > > > > > http://jenkins.kudu.apache.org/job/kudu-gerrit/23535/ : > FAILURE > > > > > > > > Looks like the build machines don't have "bc" which is used > to compare floating point numbers in bash. I'll look for > alternative or some other technique. > > > > > > This is getting to be a pretty hefty script. Before trying to > jump through more hoops to get this working, have you given much > thought to folding these calls into the tool as some `kudu master > orchestrate_add_master` or even into the `add_master` tool > directly? > > > > > > I'm fine with this being a bash script, but I want to be sure > bash is actually the right tool for the job, especially considering > we have a means to bake a lot of this into tooling fairly easily -- > heck, you've already done a lot of this in dynamic_multi_master-test! > And from a maintainability perspective, big bash scripts like seem > much more palatable when not written in bash :) > > > > If you look at series of recent changes most of them I've tried > to push into the CLI tool or the server implementation itself. > > > > Current tooling doesn't have good means for process mgmt. > ExternalMiniCluster does have that but it's not available for > production tool as I see it. That's where bash is better. > > > > The script might be lengthy but I've used linters and tried to > make it easier to read and maintain as much as possible. > > > > That being said, I'll look into more validations that could be > put into the CLI tool or the server instead of the script. I'll > explore whether more if not all could be migrated into the C++ > tool. > > Yeah, process management can be tough, but at the very least there > are some utilities in util/subprocess.h that we use to run the > tools in tests. I imagine the tricky part though is starting a > Master server, but even that seems like something we could work > around, especially if the end result of the tool is the new master > is down, and we expect CM to start the master up, which I think > would be the case anyway. > > It's your call in the end, just thought I'd raise the suggestion > because the script was getting pretty complex, and at least most of > the tools we have in the C++ codebase seem to overlap with what's > needed here. +1 I'd vote for CLI-based approach here as well, if possible. If not everything, then maybe create basic building blocks as commands in the CLI and use the scripting approach as the last resport just to glue all of these together? -- To view, visit http://gerrit.cloudera.org:8080/17220 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibc9dad73be625ff30070c522626bd79f1f1aa90c Gerrit-Change-Number: 17220 Gerrit-PatchSet: 3 Gerrit-Owner: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 29 Mar 2021 18:15:49 +0000 Gerrit-HasComments: No
