github-actions[bot] commented on code in PR #65532:
URL: https://github.com/apache/doris/pull/65532#discussion_r3595990896
##########
regression-test/suites/compaction/test_compaction_with_empty_rowset.groovy:
##########
@@ -44,7 +44,8 @@ suite("test_compaction_mow_with_empty_rowset", "p0") {
DISTRIBUTED BY HASH(`k1`) BUCKETS 2
PROPERTIES (
"replication_allocation" = "tag.location.default: 1",
- "enable_unique_key_merge_on_write" = "true"
+ "enable_unique_key_merge_on_write" = "true",
+ "disable_auto_compaction" = "true"
Review Comment:
[P1] Require a fresh reduction in the second phase
The first polling phase returns only after `rowCount < 10 * replicaNum`.
These next ten loads can add at most ten data segments per replica, so before
this second trigger the count is already strictly below `20 * replicaNum`. If
visible-version lag makes the manual request return E-2000, the helper omits
that tablet from its wait set and this predicate passes immediately without
compacting the new batch; the new property removes the automatic fallback.
Please capture the count or success timestamp immediately before this trigger
and require a fresh reduction/completion instead of comparing against the
cumulative twenty-load bound.
##########
regression-test/suites/compaction/test_vertical_compaction_agg_state.groovy:
##########
@@ -53,7 +53,7 @@ suite("test_vertical_compaction_agg_state") {
AGGREGATE KEY(`user_id`)
COMMENT 'OLAP'
DISTRIBUTED BY HASH(`user_id`) BUCKETS 1
- PROPERTIES ( "replication_num" = "1" );
+ PROPERTIES ( "replication_num" = "1", "disable_auto_compaction" =
"true" );
Review Comment:
[P1] Make this retry predicate prove that compaction ran
This table receives only four single-segment rowsets, but the default
`cumulative_compaction_min_deltas` is 5 (and these tiny rows are below the 64
MiB size override). The manual cumulative request therefore returns E-2000 even
after versions are visible; the helper ignores that tablet, and the first
Awaitility poll sees `rowCount == 4`, which already satisfies `rowCount < 8 *
replicaNum`. With this new property there is no background compaction fallback,
so the suite passes without testing vertical aggregate-state compaction. Please
create enough compaction-eligible inputs (or use the appropriate compaction
type) and compare against a captured pre-trigger count/success timestamp so the
predicate requires an actual compaction.
##########
regression-test/suites/schema_change_p0/test_schema_change_with_empty_rowset.groovy:
##########
@@ -53,7 +53,8 @@ suite("test_schema_change_with_empty_rowset",
"p0,nonConcurrent") {
DISTRIBUTED BY HASH(`k1`) BUCKETS 2
PROPERTIES (
"replication_allocation" = "tag.location.default: 1",
- "enable_unique_key_merge_on_write" = "true"
+ "enable_unique_key_merge_on_write" = "true",
+ "disable_auto_compaction" = "true"
Review Comment:
[P1] Verify compaction before using the remaining version budget
This suite caps `max_tablet_version_num` at 100, creates 100 load versions,
and then relies on one cumulative trigger before issuing 20 more loads.
`trigger_and_wait_compaction` drops E-2000/E-2010 tablets from its wait set,
and a single trigger can see/merge only an asynchronously visible prefix (the
sibling empty-rowset compaction test already retries for this). With this new
property, `RowsetBuilder`'s high-version emergency compaction is disabled too,
so a no-op or partial trigger can leave the tablet near version 101 and make
the post-ALTER inserts fail with TOO_MANY_VERSION. Please retry and assert
enough version/rowset-count headroom before starting the ALTER and subsequent
loads.
##########
regression-test/suites/compaction/test_compacation_with_delete.groovy:
##########
@@ -61,7 +61,7 @@ suite("test_compaction_with_delete") {
`max_dwell_time` INT DEFAULT "0" COMMENT "用户最大停留时间",
`min_dwell_time` INT DEFAULT "99999" COMMENT "用户最小停留时间")
DUPLICATE KEY(`user_id`, `date`, `datev2`, `datetimev2_1`,
`datetimev2_2`, `city`, `age`, `sex`) DISTRIBUTED BY HASH(`user_id`)
- BUCKETS 1 PROPERTIES ( "replication_num" = "1" );
+ BUCKETS 1 PROPERTIES ( "replication_num" = "1",
"disable_auto_compaction" = "true" );
Review Comment:
[P1] Do not let this manual-compaction test false-pass
After this property is set, the one-shot trigger at line 102 is the only
compaction path. The helper ignores E-2000/E-2010 by omitting that tablet from
`triggered_tablets`, so an empty wait set returns immediately when
visible-version propagation lags or a delete version is encountered. The final
`rowCount <= 8 * replicaNum` parses segment counts, and the four insert
segments plus zero-segment delete rowsets already satisfy it before any merge.
The same vacuous one-shot check exists in `test_compaction_dup_keys.groovy`,
`test_compaction_dup_keys_with_delete.groovy`,
`test_compaction_uniq_keys_row_store.groovy`, and
`test_vertical_compaction_dup_keys.groovy`. For E-2000 sites, retry until
inputs are visible and prove a strict reduction; for this delete-version path,
run the required base/full follow-up (or build compaction-eligible groups) and
require an observed success/reduction.
--
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]
