github-actions[bot] commented on code in PR #65032:
URL: https://github.com/apache/doris/pull/65032#discussion_r3502945034
##########
be/src/storage/tablet/tablet.cpp:
##########
@@ -1209,7 +1209,8 @@ bool Tablet::suitable_for_compaction(
CompactionType compaction_type,
std::shared_ptr<CumulativeCompactionPolicy>
cumulative_compaction_policy) {
#ifndef BE_TEST
- if (compaction_type == CompactionType::CUMULATIVE_COMPACTION &&
+ if ((compaction_type == CompactionType::CUMULATIVE_COMPACTION ||
Review Comment:
This fixes the automatic binlog scheduler path, but the PR does not add a
test that actually runs that path. The new `BINLOG_COMPACTION` branch is inside
`#ifndef BE_TEST`, so a normal BE unit test of
`find_best_tablets_to_compaction()` would not execute the policy refresh, and
the existing row-binlog regression cases disable auto compaction and call
`trigger_and_wait_compaction(..., "binlog")`, which goes through the manual
HTTP path that already refreshes the tablet policy in
`StorageEngine::submit_compaction_task()`. Please add coverage that leaves auto
binlog compaction enabled and verifies the first background binlog compaction
can run before any cumulative compaction for a tablet whose `compaction_policy`
is refreshed here, or refactor the refresh so a unit test can exercise it.
Without that, the regression fixed by this PR can slip back without any test
failing.
--
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]
