github-actions[bot] commented on code in PR #65907:
URL: https://github.com/apache/doris/pull/65907#discussion_r3868753191
##########
be/src/cloud/cloud_cumulative_compaction.cpp:
##########
@@ -26,19 +28,92 @@
#include "cloud/config.h"
#include "common/config.h"
#include "common/logging.h"
+#include "common/metrics/doris_metrics.h"
#include "common/status.h"
#include "cpp/sync_point.h"
#include "service/backend_options.h"
#include "storage/compaction/compaction.h"
#include "storage/compaction/cumulative_compaction_policy.h"
#include "storage/compaction/cumulative_compaction_time_series_policy.h"
+#include "storage/merger.h"
+#include "storage/rowset/rowset_reader.h"
+#include "storage/rowset/rowset_writer.h"
+#include "storage/tablet/tablet_schema.h"
#include "util/debug_points.h"
#include "util/trace.h"
#include "util/uuid_generator.h"
namespace doris {
using namespace ErrorCode;
+namespace cloud {
+
+bool is_single_rowset_compaction_candidate(const RowsetSharedPtr& rowset) {
+ const auto& rowset_meta = rowset->rowset_meta();
+ const int64_t overlap_unit_count =
+ rowset_meta->segments_overlap() == NONOVERLAPPING_WITHIN_GROUP
+ ?
static_cast<int64_t>(rowset_meta->segment_group_sizes().size())
+ : rowset->num_segments();
+ return !rowset_meta->has_delete_predicate() &&
rowset_meta->is_segments_overlapping() &&
+ overlap_unit_count >=
config::cloud_single_rowset_compaction_min_segments;
Review Comment:
[P2] Avoid whole-rowset horizontal fallback after grouping
After the first pass, `overlap_unit_count` switches from physical segments
to logical groups. With the defaults, a near-target 1,000-segment rowset can
retain about 1,000 physical segments in only 16 groups, so this check rejects
another grouped pass (`16 < 512`) even though the size policy still selects the
singleton by its physical compaction score. The task then falls through to
ordinary compaction; for `has_seq_map()` (or with vertical compaction
disabled), `VMergeIterator::init()` eagerly initializes all ~1,000 segment
contexts and first blocks, recreating the memory spike that `G=64` is meant to
avoid. Please keep persisted grouped rowsets on a group-aware bounded path
until they become non-overlapping, or teach horizontal reading to consume one
lazy chain per group, and add a two-task horizontal/sequence-map 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]
