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

bulk: increased split-after size can result in replica imbalance #75664

Closed
nicktrav opened this issue Jan 28, 2022 · 6 comments
Closed

bulk: increased split-after size can result in replica imbalance #75664

nicktrav opened this issue Jan 28, 2022 · 6 comments
Assignees
Labels
A-disaster-recovery C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-disaster-recovery

Comments

@nicktrav
Copy link
Collaborator

nicktrav commented Jan 28, 2022

Describe the problem

Spin-off from #68303 - provided a distilled version. Context on original, here.

It appears as though e12c9e6 causes replica imbalance which doesn't play nicely with large imports.

With that commit, on clearrange/checks=true, the import fails as a number of nodes run out of disk due to the imbalance:

Screen Shot 2022-01-28 at 9 37 24 AM

Without that commit the import succeeds:

Screen Shot 2022-01-28 at 10 47 39 AM

Jira issue: CRDB-12773

@nicktrav nicktrav added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Jan 28, 2022
@nicktrav
Copy link
Collaborator Author

cc: @dt

@blathers-crl
Copy link

blathers-crl bot commented Jan 28, 2022

cc @cockroachdb/bulk-io

@dt
Copy link
Member

dt commented Feb 2, 2022

Reverting in #75882 for now, will try to find some time next week to poke at this, and see why the closer-to-default split size does so badly / what the extremely low split size was doing that was better for the allocator.

Maybe the bigger range counts were covering for scatter's known padding issues? Might need @aayushshah15 to help me poke at this when I get to it.

@aayushshah15
Copy link
Contributor

Maybe the bigger range counts were covering for scatter's known padding issues? Might need @aayushshah15 to help me poke at this when I get to it.

I'd still expect the allocator to fix straight-up replica count divergence though. Very curious why this was happening. Feel free to put something on my calendar if you want to look together, but I can also just try repro-ing on my own with the split size bumped.

@aayushshah15
Copy link
Contributor

aayushshah15 commented Feb 3, 2022

I am playing with this a little bit (i.e. running with 384mb with our patch for AdminScatter) on a tpcc import. Repro steps are identical to what I've described in that PR, except for the increased split-after size.

The cluster is at http://104.196.120.245:26258/#/metrics/replication/cluster

The main things that I've noticed so far are that, with 384mb, we essentially stop scattering after the very beginning of the import. My sense is that this runs contrary to our understanding (also something that wasn't happening with 48mb)? Yellow is rebalances.
image

Additionally, as far as I can tell, the allocator is continuously rebalancing. The rate at which we're converging is just slower than the rate at which newer splits are being created. After the import is done, I'd expect this to fully converge.

        {
          "time": "2022-02-03T15:12:34.421667145Z",
          "message": "kv/kvserver/allocator_scorer.go:186 [n7,status] s7: should-rebalance(ranges-overfull): rangeCount=1909, mean=1291.00, overfull-threshold=1356"
        },

image

@dt
Copy link
Member

dt commented Mar 26, 2022

fixed by #77588

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-disaster-recovery C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-disaster-recovery
Projects
No open projects
Archived in project
Development

No branches or pull requests

3 participants