Andrew Wong 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. -- 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: Andrew Wong <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 24 Mar 2021 21:22:05 +0000 Gerrit-HasComments: No
