[fix](case) lower cold compaction threshold in fault injection test#65554
[fix](case) lower cold compaction threshold in fault injection test#65554shuke987 wants to merge 1 commit into
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
/review |
|
run buildall |
|
Codex automated review failed and did not complete. Error: You've hit your usage limit. Visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at Jul 20th, 2026 12:23 AM. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
|
/review |
There was a problem hiding this comment.
Automated review completed for PR 65554.
I found two blocking issues in the test update. First, the new setBeConfigTemporary path may fail before applying or restoring cold_data_compaction_score_threshold because the BE update-config handler dereferences a missing optional persist parameter. Second, once the threshold is applied, the test can still pass on a normal successful cold-data compaction path because the enabled debug point is not on the cold-data score/execute path and the object count of 7 is not unique fault evidence.
Critical checkpoint conclusions: goal/test proof is not satisfied yet because the modified regression test can fail before setup or false-pass without proving the intended fault; the change is narrow but relies on shared BE config and cold-compaction behavior; no new production concurrency was added, but the mutable process-wide config is only safe if the update/restore API works; no storage-format or rolling-upgrade compatibility change is introduced; the relevant parallel successful cold-data-compaction path was checked and overlaps the asserted object count; test coverage is present as a regression test edit, but I did not run tests or builds because this review environment explicitly forbids builds/code execution beyond review operations. User focus: no additional user-provided focus was supplied.
| def tabletName = "test_cold_data_compaction_fault_injection" | ||
|
|
||
| def customBeConfig = [ | ||
| cold_data_compaction_score_threshold : 4 |
There was a problem hiding this 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.
| cold_data_compaction_score_threshold : 4 | ||
| ] | ||
|
|
||
| setBeConfigTemporary(customBeConfig) { |
There was a problem hiding this 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.
What problem does this PR solve?
Issue Number: N/A
Related PR: #55333
Problem Summary:
test_cold_data_compaction_fault_injectioncreates five remote rowsets, but the current defaultcold_data_compaction_score_thresholdis 100. The tablet's cold-data compaction score is 5, so the producer keeps skipping it and the case times out while the remote object count stays at 6.Temporarily set
cold_data_compaction_score_thresholdto 4 for this case, matching the existingcold_heat_separation/cold_data_compaction.groovycoverage.setBeConfigTemporaryrestores the original value after the case.Release note
None
Check List (For Author)
Test
Manual validation:
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)