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

vtctld: make cell optional #6111

Merged
merged 2 commits into from
Apr 27, 2020
Merged

Conversation

sougou
Copy link
Contributor

@sougou sougou commented Apr 26, 2020

This change makes the cell flag optional for vtctld. This removes
the catch-22 that you have to create a cell before launching vtctld.
But then, how do you create a cell without a vtctld?

This required a one-time use of vtctl tool to create this initial
cell.

If you don't specify a cell, vtctld will choose one for you.
I believe the main use case for this is for identifying which shards
are serving vs. non-serving. This info is now available in global
topo itself as the "IsMasterServing" flag. But, we'll need to
go into the UI to make this change, which is non-trivial right now.

Signed-off-by: Sugu Sougoumarane [email protected]

This change makes the cell flag optional for vtctld. This removes
the catch-22 that you have to create a cell before launching vtctld.
But then, how do you create a cell without a vtctld?

This required a one-time use of vtctl tool to create this initial
cell.

If you don't specify a cell, vtctld will choose one at random.
I believe the main use case for this is for identifying which shards
are serving vs. non-serving. This info is now available in global
topo itself as the "IsMasterServing" flag. But, we'll need to
go into the UI to make this change, which is non-trivial right now.

Signed-off-by: Sugu Sougoumarane <[email protected]>
@sougou sougou requested a review from enisoc April 26, 2020 03:20
Also use the API only if localcell is not specified.

Signed-off-by: Sugu Sougoumarane <[email protected]>
@sougou sougou requested a review from deepthi April 27, 2020 04:37
@@ -287,9 +287,17 @@ func initAPI(ctx context.Context, ts *topo.Server, actions *ActionRepository, re

if cell == "local" {
if *localCell == "" {
return nil, fmt.Errorf("local cell requested, but not specified. Please set with -cell flag")
cells, err := ts.GetCellInfoNames(ctx)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that local is a reserved cell name and no one can use it for their cells? We need to document that.

@morgo
Copy link
Contributor

morgo commented Apr 27, 2020

I think this will improve the local example code a lot. Let's take a look after Vitess 6, so we can think through it from an examples perspective.

@morgo
Copy link
Contributor

morgo commented Apr 27, 2020

If you don't specify a cell, vtctld will choose one at random.

Should this say deterministically and not random? Otherwise LGTM.

Copy link
Member

@deepthi deepthi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. We need to track the documentation requirement elsewhere.

@deepthi
Copy link
Member

deepthi commented Apr 27, 2020

If you don't specify a cell, vtctld will choose one at random.

Should this say deterministically and not random? Otherwise LGTM.

I suggest vtctld will choose one for you.

@morgo morgo merged commit bec4139 into vitessio:master Apr 27, 2020
@sougou sougou deleted the ss-vtctld-nocell branch April 28, 2020 02:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants