[persist] Prioritize structured data in the batch builder #30643
Conversation
This file contains 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
bkirwi
force-pushed
the
structured-first
branch
from
ac3ed36 to
a496901
Compare
|
Partial nightlies: https://buildkite.com/materialize/nightly/builds/10551 |
bkirwi
force-pushed
the
structured-first
branch
from
a496901 to
1a335eb
Compare
ParkMyCar
approved these changes
Dec 2, 2024
| @@ -65,7 +65,7 @@ pub enum BatchColumnarFormat { | |||
| impl BatchColumnarFormat { | |||
| /// Returns a default value for [`BatchColumnarFormat`]. | |||
| pub const fn default() -> Self { | |||
| BatchColumnarFormat::Row | |||
| BatchColumnarFormat::Both(2) | |||
bkirwi
force-pushed
the
structured-first
branch
from
1a335eb to
144ec98
Compare
This requires updating the threshold in some tests... the new stuff uses the `goodbytes` estimates, which exclude some stuff that used to be included.
Previously this was split between BatchBuilder and BatchBuilderInternal; now it's all in BatchBuilder, and the lower-level code only deals with part-size chunks.
bkirwi
force-pushed
the
structured-first
branch
2 times, most recently
from
78c33ea to
5ebea69
Compare
|
Picking this up again now that we're widely rolled out, it turns out that some benchmarks are showing regressions after the final commit. I didn't have time to chase that error down before the holidays, so I'm going to slice that commit off and try to get the rest of this merged so it doesn't rot by the time I'm back... |
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.
This is a nice milestone: it's the last codepath that could only handle codec data; now both are always handled in parallel. However, we don't allow disabling codec-data writes yet!
Motivation
Part of: https://github.com/MaterializeInc/database-issues/issues/7411
Tips for reviewer
Lots of code movement... might be easiest to follow commit-by-commit.
Checklist
$T ⇔ Proto$Tmapping (possibly in a backwards-incompatible way), then it is tagged with aT-protolabel.