Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli: don't allow balancer to be specified via command line flags #4732

Closed
spencerkimball opened this issue Feb 29, 2016 · 8 comments · Fixed by #5011
Closed

cli: don't allow balancer to be specified via command line flags #4732

spencerkimball opened this issue Feb 29, 2016 · 8 comments · Fixed by #5011
Assignees
Milestone

Comments

@spencerkimball
Copy link
Member

Choosing between usage and range-count for rebalancing decisions is not something we should expect operators to specify on the command line. Let's choose the appropriate default for now and then move towards an automated approach.

@jseldess don't document the --balance_mode flag.

@petermattis petermattis changed the title Don't allow balancer to be specified via command line flags cli: don't allow balancer to be specified via command line flags Feb 29, 2016
@petermattis petermattis added this to the Beta milestone Feb 29, 2016
@mrtracy
Copy link
Contributor

mrtracy commented Feb 29, 2016

If we're going to remove this flag, that is going to have consequences for testing cockroach with multiple nodes on a single machine (which is the reason this flag was added in the first place). There are a few ways to do this:

  1. Declare that "rangecount" is the new default. I'm fine with this.
  2. Allow stores to be specified with a maximum size that is independent of the disk size. You would also need to update the way that a store's Capacity is computed to account for this. In this case, "usage" can remain the default and the single-node scenario still works.
  3. Declare that running multiple nodes on a single machine is not a valid testing scenario for the purposes of rebalancing - I don't like this scenario, but it's possible.

Additionally, I don't know if we'll ever have an "automated approach" for this use case; this flag was intended for developers to try out alternative balance strategies, and while it initially is only used to solve the "multi-node, same hard drive" conundrum, it could eventually allow developers to compare different algorithms without needing special builds. Instead, I think it should moved to gossiped cluster configuration; the main problem with this flag is that it's a cluster-wide setting, not a per-node setting, and thus makes no sense as a command line argument.

@bdarnell
Copy link
Contributor

@BramGruneir is working on the second option, to be able to cap the size of each store.

@spencerkimball
Copy link
Member Author

@mrtracy again, I'm trying to avoid having @jseldess explain what these flags mean. And once we release a flag in the wild, we're going to have a much more difficult time removing it.

@mrtracy
Copy link
Contributor

mrtracy commented Mar 1, 2016

@spencerkimball I'm fine with removing the flag, we just need to pick an alternative solution for the problem it was solving. It looks like we're going to go with solution #2.

@BramGruneir
Copy link
Member

Just a head's up that the maximum size work will be done by end of week.

@spencerkimball
Copy link
Member Author

@BramGruneir should this be closed?

@BramGruneir
Copy link
Member

As soon as I get the size changed merged.
The code should be ready to go this evening.

On Sun, Mar 6, 2016 at 1:26 PM, Spencer Kimball notifications@github.com
wrote:

@BramGruneir https://github.com/BramGruneir should this be closed?


Reply to this email directly or view it on GitHub
#4732 (comment)
.

@jseldess
Copy link
Contributor

jseldess commented Mar 7, 2016

--store=size documented here: https://www.cockroachlabs.com/docs/start-a-node.html#store

@mrtracy mrtracy assigned BramGruneir and unassigned mrtracy Mar 8, 2016
BramGruneir added a commit to BramGruneir/cockroach that referenced this issue Mar 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants