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

release-19.1: storage: remove dependency from replication queue to the split queue #38588

Merged

Conversation

andreimatei
Copy link
Contributor

Backport 1/1 commits from #38529.

/cc @cockroachdb/release


Before this patch, the replication queue would refuse to process ranges
that it thought needed splits because of their size. The idea was to let
the split queue process them first. There's no reason for this,
particularly since we've introduced the write-backpressuring mechanism
which keeps ranges from getting too large.
The check had not considered unsplittable ranges (i.e. ranges with only
(versions of) one row or some system ranges). These can't be split but
the replication queue was disconsidering them none the less.

Release note (bug fix): Ranges consisting of only one row (and
historical versions of that row) are now correctly up-replicating.

@andreimatei andreimatei requested review from knz, nvanbenschoten and a team July 1, 2019 18:29
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@andreimatei andreimatei removed the request for review from knz July 1, 2019 18:29
@tbg
Copy link
Member

tbg commented Jul 1, 2019 via email

@andreimatei
Copy link
Contributor Author

andreimatei commented Jul 1, 2019 via email

Copy link
Contributor Author

@andreimatei andreimatei left a comment

Choose a reason for hiding this comment

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

a night or two have passed :P

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @nvanbenschoten)

@tbg
Copy link
Member

tbg commented Jul 8, 2019

LGTM

@andreimatei
Copy link
Contributor Author

bors r+

@craig
Copy link
Contributor

craig bot commented Jul 8, 2019

Build failed

Before this patch, the replication queue would refuse to process ranges
that it thought needed splits because of their size. The idea was to let
the split queue process them first. There's no reason for this,
particularly since we've introduced the write-backpressuring mechanism
which keeps ranges from getting too large (although not all requests are
backpressured, so theoretically ranges could still grow larger than we'd
like).
The check had not considered unsplittable ranges (i.e. ranges with only
(versions of) one row or some system ranges). These can't be split but
the replication queue was disconsidering them none the less.

This change was motivated by the unsplittable SystemConfigSpan range.
This kind of system range is not backpressured, but we still don't want
the replication queue to dismiss it when it's too large (since, again,
it can't be split).

Release note (bug fix): Ranges consisting of only one row (and
historical versions of that row) are now correctly up-replicating.
Copy link
Contributor Author

@andreimatei andreimatei left a comment

Choose a reason for hiding this comment

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

I've removed the test from this backport. It doesn't work as written because the way to change default zone configs was different before 19.2. I don't think it's worth fixing.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @nvanbenschoten)

@andreimatei andreimatei merged commit f74db5e into cockroachdb:release-19.1 Jul 9, 2019
@andreimatei andreimatei deleted the backport19.1-38529 branch July 9, 2019 18:34
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