release-21.1: improve restore performance consistency#64656
Merged
pbardea merged 4 commits intocockroachdb:release-21.1from May 17, 2021
Merged
release-21.1: improve restore performance consistency#64656pbardea merged 4 commits intocockroachdb:release-21.1from
pbardea merged 4 commits intocockroachdb:release-21.1from
Conversation
This commit filters out importSpans that have no associated data. File spans' start keys are set to be the .Next() of the EndKey of the previous file span. This was at least the case on the backup that is currently being run against restore2TB. This lead to a lot of importSpan entries that contained no files and would cover an empty key range. The precense of these import spans did not effect the performance of restores that much, but they did cause unnecessary splits and scatters which further contributed to the elevated NLHEs that were seen. This commit generally calms down the leaseholder errors, but do not eliminate them entirely. Future investigation is still needed. It also does not seem to have any performance impact on RESTORE performance (in terms of speed of the job). Release note: None
This change reworks how RESTORE splits and scatter's in 2 ways: 1. This commits updates the split and scatter mechanism that restore uses to split at the key of the next chunk/entry rather than at the current chunk/entry. This resolves a long-standing TODO that updates the split and scattering of RESTORE to perform a split at the key of the next chunk/entry. Previously, we would split at a the start key of the span, and then scatter that span. Consider a chunk with entries that we want to split and scatter. If we split at the start of each entry and scatter the entry we just split off (the right hand side of the split), we'll be continuously scattering the suffix keyspace of the chunk. It's preferrable to split out a prefix first (by splitting at the key of the next chunk) and scattering the prefix. We additionally refactor the split and scatter processor interface and stop scattering the individual entries, but rather just split them. 2. Individual entries are no longer scattered. They used to be scattered with the randomizeLeases option set to false, but there was not any indication that this was beneficial to performance, so it was removed for simplicity. Release note: None
This change fixes a bug in the planning of the restore DistSQL flow. The ordering of the source router slots did not match the order that the streams were added to the range router. This meant that the range router could mis-direct rows in the flow to the incorrect node. Ever after this change, it appears that a lot of AddSSTables are not handled locally and so the release note does not indicate any performance improvement. Release note: None
This change updates the chunk size to take larger clusters into account. On larger clusters, having more chunks (that are each smaller) is beneficial because this results in more even load after scattering these chunks. Release note: None
Member
Contributor
Author
|
TFTR! |
craig bot
pushed a commit
that referenced
this pull request
May 17, 2021
64656: release-21.1: improve restore performance consistency r=dt a=pbardea Backport: * 2/2 commits from "backupccl: rework split and scatter mechanism" (#63471) * 1/1 commits from "backupccl: fix a bug in routing scattered ranges" (#63875) * 1/1 commits from "backupccl: create more chunks on larger clusters" (#64067) Please see individual PRs for details. /cc @cockroachdb/release Added do-not-merge label until release-21.1 opens for 21.1.1 backports once the release is out. Co-authored-by: Paul Bardea <pbardea@gmail.com>
Contributor
|
This PR was included in a batch that successfully built, but then failed to merge into release-21.1 (it was a non-fast-forward update). It will be automatically retried. |
Contributor
Author
|
bors r- |
Contributor
|
Canceled. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport:
Please see individual PRs for details.
/cc @cockroachdb/release
Added do-not-merge label until release-21.1 opens for 21.1.1 backports once the release is out.