FrankChen021 opened a new pull request, #20306:
URL: https://github.com/apache/druid/pull/20306

   ### Description (Cause)
   
   `PartialCompactionTest` runs three rounds of parallel indexing/compaction 
back-to-back (a hash- or range-partitioned load, a dynamic-partitioned append, 
then a `MinorCompactionInputSpec` compaction task), each going through its own 
determine-partitions/generate/merge phases. Every phase transition is detected 
by `TaskMonitor`'s fixed-rate poll (`ParallelIndexPhaseRunner` → 
`TaskMonitor#start(taskStatusCheckPeriodMs)`), so each transition costs up to 
one full poll period.
   
   `AbstractParallelIndexSupervisorTaskTest#newTuningConfig` only shortens that 
period to 100ms for serial runs (`maxNumConcurrentSubTasks == 1`). 
`PartialCompactionTest` always uses `maxNumConcurrentSubTasks = 2`, so it falls 
back to the **1000ms production default**. Across the repeated multi-phase 
rounds, that polling latency dominates the test's wall-clock time (~42s for the 
slowest method in CI) without contributing to what is being asserted.
   
   ### Fix
   
   Override `newTuningConfig` in `PartialCompactionTest` to always use 
`withTaskStatusCheckPeriodMs(100L)`, regardless of concurrency. The override 
mirrors the base builder exactly (same `MaxSizeSplitHintSpec(null, 1)`, 
`maxParseExceptions(5)`, etc.); only the poll interval differs. The change is 
scoped to this class — other `AbstractMultiPhaseParallelIndexingTest` 
subclasses are untouched.
   
   ### Why this is safe
   
   - No assertion in this class depends on poll cadence; they check final 
segment sets, atomic-update-group sizes, and upgraded-segment-id mappings.
   - The test injects 20% random subtask kills and API failures 
(`DEFAULT_TRANSIENT_TASK_FAULT_RATE`/`DEFAULT_TRANSIENT_API_FAILURE_RATE`). 
Polling faster does not change that behavior: subtask retries are 
**count-based** (`TaskMonitor#maxRetry`, `numTries() < maxRetry`) and API 
retries are capped by `MAX_TRANSIENT_API_FAILURES`, not by elapsed time. A 
shorter poll only detects a kill sooner.
   - Task payloads, partitioning strategies, and expected results are unchanged.
   
   ### Benchmark
   
   Baseline: surefire report from the latest successful `apache/druid` master 
CI run 
([34215287115](https://github.com/apache/druid/actions/runs/34215287115), 
2026-09-08). After: CI run of this exact change on a fork 
([34353490634](https://github.com/FrankChen021/druid/actions/runs/34353490634), 
JDK 25). Surefire `<testcase time>`:
   
   | Test | Before | After | Change |
   |---|---:|---:|---:|
   | `testPartialCompactRangeAndDynamicPartitionedSegments` | 42.00s | 
**7.31s** | −83% |
   | `testPartialCompactHashAndDynamicPartitionedSegments` | 35.54s | **5.71s** 
| −84% |
   | `testMinorCompactionUpgradesNonCompactedSegments` | 24.13s | **3.85s** | 
−84% |
   
   All tests in the class pass with unchanged assertions.
   
   <hr>
   
   ##### Key changed/added classes in this PR
    * `PartialCompactionTest`
   
   <hr>
   
   This PR has:
   
   - [x] been self-reviewed.
   - [x] added comments explaining the "why" and the intent of the code 
wherever would not be obvious for an unfamiliar reader.
   - [x] added or modified existing tests to cover new code paths (test-only 
change; no production code touched).


-- 
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]

Reply via email to