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

split: [DNM] only retain safe split keys #103565

Closed
wants to merge 1 commit into from

Conversation

kvoli
Copy link
Collaborator

@kvoli kvoli commented May 18, 2023

Previously, the load based range splitter could suggest split keys which were in-between SQL rows. This violated assumptions made in SQL code, which require that rows are never split across ranges.

This patch updates the unweighted split finder to only retain safe sample keys. The weighted split finder already does this (e4f003b). Note that the weighted split finder is used by default (>=23.1), whilst the unweighted split finder is used when
kv.allocator.load_based_rebalancing.objective is set to qps (default cpu).

Informs: #43094
Fixes: #103483

Release note: None

Previously, the load based range splitter could suggest split keys which
were in-between SQL rows. This violated assumptions made in SQL code,
which require that rows are never split across ranges.

This patch updates the unweighted split finder to only retain safe
sample keys. The weighted split finder already does this (e4f003b). Note
that the weighted split finder is used by default (>=23.1), whilst the
unweighted split finder is used when
`kv.allocator.load_based_rebalancing.objective` is set to `qps` (default
`cpu`).

Informs: cockroachdb#43094
Fixes: cockroachdb#103483

Release note: None
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@kvoli
Copy link
Collaborator Author

kvoli commented May 22, 2023

closing in favor of #103690

@kvoli kvoli closed this May 22, 2023
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.

kv: load-based splitter permits splits between SQL rows, given non-SQL row inputs
2 participants