github-actions[bot] commented on code in PR #65554:
URL: https://github.com/apache/doris/pull/65554#discussion_r3588248004
##########
regression-test/suites/fault_injection_p0/test_cold_data_compaction_fault_injection.groovy:
##########
@@ -39,6 +39,11 @@ suite("test_cold_data_compaction_fault_injection",
"nonConcurrent") {
}
def tabletName = "test_cold_data_compaction_fault_injection"
+ def customBeConfig = [
+ cold_data_compaction_score_threshold : 4
Review Comment:
Lowering the threshold does make the producer submit this tablet, but it
also means the test can pass without exercising the intended fault. The
producer uses `calc_cold_data_compaction_score()` and skips only `score <
cold_data_compaction_score_threshold`; with five cooled DUP-key rowsets the
score is 5, so threshold 4 admits the tablet. The enabled debug point is in
`_calc_cumulative_compaction_score()`, which is not used by the cold-data
producer or by `ColdDataCompaction::prepare_compact()/execute_compact()`. A
normal successful cold compaction can still leave the five old remote rowset
objects plus one new output rowset plus the stable meta object until remote
cleanup runs, so the `size() == 7` wait can succeed on the non-fault path.
Please inject a point that is actually reached by cold-data compaction after
the output is written, or assert a state that cannot occur after a successful
cold compaction.
##########
regression-test/suites/fault_injection_p0/test_cold_data_compaction_fault_injection.groovy:
##########
@@ -39,6 +39,11 @@ suite("test_cold_data_compaction_fault_injection",
"nonConcurrent") {
}
def tabletName = "test_cold_data_compaction_fault_injection"
+ def customBeConfig = [
+ cold_data_compaction_score_threshold : 4
+ ]
+
+ setBeConfigTemporary(customBeConfig) {
Review Comment:
This new wrapper relies on `/api/update_config` before the test can lower
the threshold, but the helper posts only `?<param>=<value>` and omits
`persist`. In the current BE handler, `persist` is described as optional, yet
`handle_update_config()` unconditionally does
`req->params()->find("persist")->second.compare("true")` as soon as any query
parameter exists. Since `set_be_param()` sends
`/api/update_config?cold_data_compaction_score_threshold=4`, the handler can
dereference `end()` before applying the config, and the `finally` restore path
uses the same request shape. Please either make the update handler use
`req->param("persist")`/an iterator check, or include `persist=false` in the
regression helper before relying on this wrapper here.
--
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]
