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.

Sign up for GitHub

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

Jump to bottom

ui: data distribution screen missing some zone configs #27718

Closed
vilterp opened this issue Jul 18, 2018 · 1 comment
Closed

ui: data distribution screen missing some zone configs #27718

vilterp opened this issue Jul 18, 2018 · 1 comment
Assignees
@vilterp
Labels
A-webui-data-distribution C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption.

Comments

@vilterp
Copy link
Contributor

vilterp commented Jul 18, 2018

Repro

  1. Follow the steps in the Movr README to set up a partitioned cluster, including setting up 9 zone configs (3 tables x 3 regions)
  2. Look at the data distribution page

Expected

See all 9 zone configs you just set up

Actual

Only see three of the zone configs you just set up (3 tables x 1 region):

image
@vilterp vilterp self-assigned this Jul 18, 2018
@vilterp vilterp added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption. A-webui-data-distribution labels Jul 18, 2018
@vilterp vilterp mentioned this issue Jul 18, 2018
Closed
@vilterp
Copy link
Contributor Author

vilterp commented Oct 15, 2018

Oof, it's because partition zone configs have the same id (probably because they are stored subzones); getting overwritten here:

cockroach/pkg/server/admin.go

Line 1557 in a6c788a

resp.ZoneConfigs[zcID] = serverpb.DataDistributionResponse_ZoneConfig{

@vilterp vilterp mentioned this issue Oct 15, 2018
Merged
craig bot pushed a commit that referenced this issue Oct 16, 2018
Merge #31407
fc334d3 
31407: server(ccl)/ui: identify zone configs by zone name, not ID r=vilterp a=vilterp

Previously, the `DataDistribution` endpoint was returning a map of zone configs by zone config ID. However, zone config ID is not a unique identier within `crdb_internal.zones`, since subzones attached to partitions share the same ID (that of the top-level zone config), so map entries were being overwritten and the endpoint was returning an incomplete list.

Zone names should be unique; this change uses those instead and updates the UI accordingly.

This change also introduces a `serverccl` package for the unit test to live in, since testing this code path requires creating partitions, a CCL feature.

Fixes #27718

Release note: None

Co-authored-by: Pete Vilter <[email protected]>
@craig craig bot closed this as completed in #31407 Oct 16, 2018
@vilterp vilterp mentioned this issue Oct 16, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@vilterp vilterp

Labels
A-webui-data-distribution C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@vilterp