storage: compaction alignment test
#24636
Merged
+283
−3
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
WillemKauf
changed the title
storage: compaction alignment test
Lazin
previously approved these changes
Dec 20, 2024
Lazin
reviewed
Dec 20, 2024
WillemKauf
force-pushed
the
e_compaction_alignment_test
branch
from
939053a to
80b0f72
Compare
CI test resultstest results on build#60030
test results on build#60036
test results on build#60040
test results on build#60545
test results on build#60564
test results on build#60596
|
WillemKauf
force-pushed
the
e_compaction_alignment_test
branch
2 times, most recently
from
6a7731e to
d6833b7
Compare
Retry command for Build#60040please wait until all jobs are finished before running the slash command |
WillemKauf
commented
Dec 23, 2024
| ss::future<ss::stop_iteration> operator()(model::record_batch rb) { | ||
| static const auto translation_batches | ||
| = model::offset_translator_batch_types(); | ||
| if ( |
There was a problem hiding this comment.
A different condition than if(rb.header().type != model::record_batch_type::raft_data).
Otherwise, compaction placeholder batches may find their way into the ot_state with a gap_length of 0.
WillemKauf
force-pushed
the
e_compaction_alignment_test
branch
from
d6833b7 to
c10d572
Compare
dotnwat
reviewed
Jan 3, 2025
Comment on lines
+35
to
+62
| static model::record_batch make_random_batch( | ||
| model::offset offset, | ||
| bool empty, | ||
| model::record_batch_type type, | ||
| std::vector<std::optional<ss::sstring>> keys, | ||
| std::vector<std::optional<ss::sstring>> values, | ||
| int num_records) { | ||
| BOOST_REQUIRE(keys.size() == values.size()); | ||
| storage::record_batch_builder builder(type, offset); | ||
| auto to_iobuf = [](std::optional<ss::sstring> x) { | ||
| std::optional<iobuf> result; | ||
| if (x.has_value()) { | ||
| iobuf buf; | ||
| buf.append(x->data(), x->size()); | ||
| result = std::move(buf); | ||
| } | ||
| return result; | ||
| }; | ||
| if (!empty) { | ||
| for (int i = 0; i < num_records; i++) { | ||
| auto key = random_generators::random_choice(keys); | ||
| auto val = random_generators::random_choice(values); | ||
| builder.add_raw_kv(to_iobuf(key), to_iobuf(val)); | ||
| } | ||
| } | ||
| return std::move(builder).build(); | ||
| } |
There was a problem hiding this comment.
is this doing something specific to the test? pretty sure we have at least a few versions of random batch creation utilities in the tree
There was a problem hiding this comment.
Doesn't look like we have another random batch builder with the same signature as the one written here.
It's possible we could use some existing batch building utilities, but Evgeny probably added this for ease of use in this test. We don't assert on values, but the key generation is obviously important for compaction here.
Lazin
previously approved these changes
Jan 9, 2025
|
@WillemKauf sorry I think I caused this conflict and it needs a rebase. |
WillemKauf
force-pushed
the
e_compaction_alignment_test
branch
from
c10d572 to
f89c04f
Compare
|
Force push to:
|
Retry command for Build#60545please wait until all jobs are finished before running the slash command |
WillemKauf
force-pushed
the
e_compaction_alignment_test
branch
from
f89c04f to
4c0afb1
Compare
|
Force push to:
|
WillemKauf
force-pushed
the
e_compaction_alignment_test
branch
from
4c0afb1 to
b37169a
Compare
The test uses different segment layouts, runs compaction and compares results.
WillemKauf
force-pushed
the
e_compaction_alignment_test
branch
from
b37169a to
1edec69
Compare
|
Force push to:
|
Lazin
approved these changes
Jan 11, 2025
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.
Minor enhancements to #24621
Backports Required
Release Notes