Skip to content

Commit 89e4675

Browse files
liaoxin01liaoxin01
committed
fix comment
1 parent 10a4293 commit 89e4675

5 files changed

Lines changed: 57 additions & 13 deletions

File tree

‎be/src/storage/compaction/compaction.cpp‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -419,13 +419,15 @@ Status CompactionMixin::do_compact_ordered_rowsets() {
419419
// link data to new rowset
420420
auto seg_id = 0;
421421
bool segments_key_bounds_truncated {false};
422+
bool any_input_aggregated {false};
422423
std::vector<KeyBoundsPB> segment_key_bounds;
423424
std::vector<uint32_t> num_segment_rows;
424425
for (auto rowset : _input_rowsets) {
425426
RETURN_IF_ERROR(rowset->link_files_to(tablet()->tablet_path(),
426427
_output_rs_writer->rowset_id(), seg_id));
427428
seg_id += rowset->num_segments();
428429
segments_key_bounds_truncated |= rowset->is_segments_key_bounds_truncated();
430+
any_input_aggregated |= rowset->rowset_meta()->is_segments_key_bounds_aggregated();
429431
std::vector<KeyBoundsPB> key_bounds;
430432
RETURN_IF_ERROR(rowset->get_segments_key_bounds(&key_bounds));
431433
segment_key_bounds.insert(segment_key_bounds.end(), key_bounds.begin(), key_bounds.end());
@@ -445,8 +447,12 @@ Status CompactionMixin::do_compact_ordered_rowsets() {
445447
rowset_meta->set_segments_overlap(NONOVERLAPPING);
446448
rowset_meta->set_rowset_state(VISIBLE);
447449
rowset_meta->set_segments_key_bounds_truncated(segments_key_bounds_truncated);
448-
bool aggregate_key_bounds = config::enable_aggregate_non_mow_key_bounds &&
449-
!_tablet->enable_unique_key_merge_on_write();
450+
// If any input was already aggregated we have no way to recover per-segment
451+
// bounds, so force aggregation on the output to keep the layout consistent
452+
// with `num_segments` / the aggregated flag, even if the config is off now.
453+
bool aggregate_key_bounds =
454+
any_input_aggregated || (config::enable_aggregate_non_mow_key_bounds &&
455+
!_tablet->enable_unique_key_merge_on_write());
450456
rowset_meta->set_segments_key_bounds(segment_key_bounds, aggregate_key_bounds);
451457
rowset_meta->set_num_segment_rows(num_segment_rows);
452458

‎be/src/storage/rowset/beta_rowset_writer.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,12 +94,12 @@ void build_rowset_meta_with_spec_field(RowsetMeta& rowset_meta,
9494
rowset_meta.set_rowset_state(spec_rowset_meta.rowset_state());
9595
rowset_meta.set_segments_key_bounds_truncated(
9696
spec_rowset_meta.is_segments_key_bounds_truncated());
97-
rowset_meta.set_segments_key_bounds_aggregated(
98-
spec_rowset_meta.is_segments_key_bounds_aggregated());
9997
std::vector<KeyBoundsPB> segments_key_bounds;
10098
spec_rowset_meta.get_segments_key_bounds(&segments_key_bounds);
101-
// Source is already aggregated (size 1 entry) or per-segment; copy verbatim.
102-
rowset_meta.set_segments_key_bounds(segments_key_bounds);
99+
// Preserve source layout: if source was aggregated (size 1), re-aggregating
100+
// the single entry is a no-op that also keeps the flag consistent.
101+
rowset_meta.set_segments_key_bounds(segments_key_bounds,
102+
spec_rowset_meta.is_segments_key_bounds_aggregated());
103103
std::vector<uint32_t> num_segment_rows;
104104
spec_rowset_meta.get_num_segment_rows(&num_segment_rows);
105105
rowset_meta.set_num_segment_rows(num_segment_rows);

‎be/src/storage/rowset/rowset_meta.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,8 @@ int64_t RowsetMeta::segment_file_size(int seg_id) const {
292292
void RowsetMeta::set_segments_key_bounds(const std::vector<KeyBoundsPB>& segments_key_bounds,
293293
bool aggregate_into_single) {
294294
_rowset_meta_pb.clear_segments_key_bounds();
295-
if (aggregate_into_single && !segments_key_bounds.empty()) {
295+
bool did_aggregate = aggregate_into_single && !segments_key_bounds.empty();
296+
if (did_aggregate) {
296297
const std::string* overall_min = &segments_key_bounds.front().min_key();
297298
const std::string* overall_max = &segments_key_bounds.front().max_key();
298299
for (const KeyBoundsPB& key_bounds : segments_key_bounds) {
@@ -306,13 +307,13 @@ void RowsetMeta::set_segments_key_bounds(const std::vector<KeyBoundsPB>& segment
306307
KeyBoundsPB* aggregated = _rowset_meta_pb.add_segments_key_bounds();
307308
aggregated->set_min_key(*overall_min);
308309
aggregated->set_max_key(*overall_max);
309-
set_segments_key_bounds_aggregated(true);
310310
} else {
311311
for (const KeyBoundsPB& key_bounds : segments_key_bounds) {
312312
KeyBoundsPB* new_key_bounds = _rowset_meta_pb.add_segments_key_bounds();
313313
*new_key_bounds = key_bounds;
314314
}
315315
}
316+
set_segments_key_bounds_aggregated(did_aggregate);
316317

317318
int32_t truncation_threshold = config::segments_key_bounds_truncation_threshold;
318319
if (config::random_segments_key_bounds_truncation) {

‎be/src/storage/tablet/base_tablet.cpp‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -463,11 +463,18 @@ Status BaseTablet::lookup_row_key(const Slice& encoded_key, TabletSchema* latest
463463
std::vector<KeyBoundsPB> segments_key_bounds;
464464
rs->rowset_meta()->get_segments_key_bounds(&segments_key_bounds);
465465
int num_segments = cast_set<int>(rs->num_segments());
466-
// MOW lookup requires per-segment bounds. Aggregation must be disabled for MOW writers.
467-
DCHECK(!rs->rowset_meta()->is_segments_key_bounds_aggregated())
468-
<< "MOW rowset unexpectedly has aggregated key bounds, rowset_id="
469-
<< rs->rowset_id().to_string();
470-
DCHECK_EQ(segments_key_bounds.size(), num_segments);
466+
// MOW lookup requires per-segment bounds. Aggregation must be disabled
467+
// for MOW writers, but enforce at runtime too — indexing segments_key_bounds[j]
468+
// below would be out-of-bounds otherwise.
469+
if (UNLIKELY(rs->rowset_meta()->is_segments_key_bounds_aggregated() ||
470+
static_cast<int>(segments_key_bounds.size()) != num_segments)) {
471+
return Status::InternalError(
472+
"MOW lookup got rowset with inconsistent segments_key_bounds, rowset_id={}, "
473+
"aggregated={}, bounds_size={}, num_segments={}",
474+
rs->rowset_id().to_string(),
475+
rs->rowset_meta()->is_segments_key_bounds_aggregated(),
476+
segments_key_bounds.size(), num_segments);
477+
}
471478
std::vector<uint32_t> picked_segments;
472479
for (int j = num_segments - 1; j >= 0; j--) {
473480
if (_key_is_not_in_segment(key_without_seq, segments_key_bounds[j],

‎be/test/storage/rowset/rowset_meta_test.cpp‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,36 @@ TEST_F(RowsetMetaTest, TestSegmentsKeyBoundsAggregation) {
389389
EXPECT_EQ(out[0].max_key(), "c00");
390390
EXPECT_TRUE(rs_meta.is_segments_key_bounds_aggregated());
391391
}
392+
393+
// 5. aggregated flag must be reset when switching from aggregate=true to
394+
// aggregate=false on the same instance.
395+
{
396+
RowsetMeta rs_meta;
397+
rs_meta.set_segments_key_bounds(per_segment, /*aggregate_into_single=*/true);
398+
ASSERT_TRUE(rs_meta.is_segments_key_bounds_aggregated());
399+
400+
rs_meta.set_segments_key_bounds(per_segment, /*aggregate_into_single=*/false);
401+
EXPECT_FALSE(rs_meta.is_segments_key_bounds_aggregated());
402+
403+
std::vector<KeyBoundsPB> out;
404+
rs_meta.get_segments_key_bounds(&out);
405+
EXPECT_EQ(out.size(), per_segment.size());
406+
}
407+
408+
// 6. aggregated flag must be reset when calling with aggregate=true but an
409+
// empty input after a prior aggregated call.
410+
{
411+
RowsetMeta rs_meta;
412+
rs_meta.set_segments_key_bounds(per_segment, /*aggregate_into_single=*/true);
413+
ASSERT_TRUE(rs_meta.is_segments_key_bounds_aggregated());
414+
415+
rs_meta.set_segments_key_bounds({}, /*aggregate_into_single=*/true);
416+
EXPECT_FALSE(rs_meta.is_segments_key_bounds_aggregated());
417+
418+
std::vector<KeyBoundsPB> out;
419+
rs_meta.get_segments_key_bounds(&out);
420+
EXPECT_TRUE(out.empty());
421+
}
392422
}
393423

394424
TEST_F(RowsetMetaTest, TestSegmentsKeyBoundsAggregationTruncation) {

0 commit comments

Comments
 (0)