github-actions[bot] commented on code in PR #65907:
URL: https://github.com/apache/doris/pull/65907#discussion_r3630165138
##########
be/src/cloud/cloud_cumulative_compaction.cpp:
##########
@@ -554,6 +601,91 @@ Status
CloudCumulativeCompaction::pick_rowsets_to_compact() {
return Status::OK();
}
+Status
CloudCumulativeCompaction::prepare_merge_input_rowsets(MergeInputRowsetsResult*
result) {
+ if (!_single_rowset_compaction_segment_group_size.has_value()) {
+ return Status::OK();
+ }
+
+ const int64_t segment_group_size =
*_single_rowset_compaction_segment_group_size;
+ if (segment_group_size <= 0) {
+ return Status::InvalidArgument(
+ "cloud_single_rowset_compaction_segment_group_size must be
positive, value={}",
+ segment_group_size);
+ }
+ result->is_segment_grouped = true;
+ result->segment_group_size = segment_group_size;
+ return Status::OK();
+}
+
+Status CloudCumulativeCompaction::do_merge_input_rowsets(
+ const std::vector<RowsetReaderSharedPtr>& input_rs_readers,
+ MergeInputRowsetsResult* result) {
+ if (!result->is_segment_grouped) {
+ return Compaction::do_merge_input_rowsets(input_rs_readers, result);
+ }
+
+ const int64_t segment_group_size = result->segment_group_size;
+ const auto& input_rowset = _input_rowsets.front();
+ const int64_t segment_group_count =
+ (input_rowset->num_segments() + segment_group_size - 1) /
segment_group_size;
Review Comment:
[P2] Require every grouped pass to reduce logical groups
For an already-grouped input, eligibility counts
segment_group_sizes().size(), but this calculation repartitions raw physical
segments. With group_size=2, an eligible layout {2,2,1} has five physical
segments and three logical groups, so it creates the same three ranges and can
reproduce {2,2,1} indefinitely at the unchanged cumulative point. group_size=1
is the same deterministic problem for raw input. Please enter this path only
when the planned output-group count is strictly below the input overlap-unit
count, or explicitly coalesce existing logical groups, and keep the setting
constant in a next-cycle test.
##########
be/src/cloud/cloud_cumulative_compaction.cpp:
##########
@@ -246,10 +276,13 @@ Status CloudCumulativeCompaction::modify_rowsets() {
// calculate new cumulative point
int64_t input_cumulative_point = cloud_tablet()->cumulative_layer_point();
auto compaction_policy =
cloud_tablet()->tablet_meta()->compaction_policy();
- int64_t new_cumulative_point =
- _engine.cumu_compaction_policy(compaction_policy)
- ->new_cumulative_point(cloud_tablet(), _output_rowset,
_last_delete_version,
- input_cumulative_point);
+ int64_t new_cumulative_point = input_cumulative_point;
+ if (!_single_rowset_compaction_segment_group_size.has_value()) {
Review Comment:
[P2] Advance the cumulative point after the final group
This branches on whether grouped mode was selected, not on the output
layout. If the configured range covers all input segments (or only one range
emits output), update_output_rowset_after_build() leaves the result as ordinary
NONOVERLAPPING, yet this condition still skips new_cumulative_point(). A
promotion-sized result then remains in the cumulative layer; the next
size-policy pass rejects the sole non-overlapping rowset, so an idle tablet
cannot advance the boundary and base compaction cannot consume it. Please
suppress advancement only when the built output actually remains
NONOVERLAPPING_WITHIN_GROUP, and add a promotion-sized one-group test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
