xiangfu0 opened a new pull request, #19199:
URL: https://github.com/apache/pinot/pull/19199
## Summary
The Unit Tests workflow currently takes ~63 min wall-clock, with ~83% of
that spent in a single-threaded `mvn test` step. This PR keeps the existing
2-shard matrix but parallelizes tests *within* each shard using surefire
fork-level parallelism, with fork-safe port/temp-dir isolation so concurrent
forks don't collide.
All new knobs default to the prior behavior, so local/dev builds are
unchanged; only CI opts into parallelism.
## What changed
**Parallelism knobs (pom.xml, overridable, default = prior behavior)**
- `unit.test.fork.count` (default `1`) → surefire `forkCount`. With
`reuseForks=false`, each fork runs one test *class* in its own JVM, so this is
process-level isolation (not TestNG intra-JVM threading) — tests that were
unsafe multi-threaded within a single JVM are unaffected. CI sets `3`.
- `unit.test.fork.heap` (default `4g`) → per-fork `-Xms/-Xmx`. CI lowers
this to `2500m` so `3 forks * 2500m + the Maven JVM` stays within the runner's
RAM.
- `unit.test.rerun.count` (default `0`) → `rerunFailingTestsCount`. Left at
`0`; kept as an escape hatch so real failures are never masked.
- `jacoco.exec.suffix` (default empty) → per-fork exec file. The unit-test
script sets it to `-${surefire.forkNumber}` so parallel forks each write a
distinct `jacoco-<n>.exec` instead of corrupting one shared file; coverage is
then aggregated via a `jacoco-*.exec` glob.
- `surefire.forkNumber` exposed as a system property so tests can offset
resources per fork.
**Fork-safe resource isolation**
- `ZkStarter`: per-fork ZK test-port offset derived from
`surefire.forkNumber` (production-inert — the property is absent outside a
surefire fork, so it falls back to the historical default port). ZK data dir
now uses a random UUID instead of `System.currentTimeMillis()` (which collides
when two instances start in the same ms).
- `ControllerTest`: per-fork controller/ZK port offset; data/temp dirs
switched to UUID-based names.
**Fork-unsafe test fixes (pinot-segment-local)**
- `DictionariesTest` / `DictionaryOptimiserTest` /
`SegmentLocalFSDirectoryTest`: these derived their index dir from another
test's class name, so they shared a directory. Switched to per-run UUID dirs
under the temp dir.
**Root-caused flakes (not masked by retry)**
- `SegmentPreProcessorTest`: assert the no-op index recreation doesn't move
the mtime by more than filesystem timestamp granularity, instead of requiring
exact equality (flaky under CPU load).
- `LuceneMutableTextIndexTest`: replace the fixed `Thread.sleep(100)` with
polling on an analyzer-independent refresh sentinel until the async NRT refresh
is visible (up to a generous timeout).
**CI script (`.pinot_tests_unit.sh`)**
- Both shards run plain `mvn test` (modules already built/installed by
`.pinot_tests_build.sh`).
- Env-overridable `UNIT_TEST_FORK_COUNT` (3), `UNIT_TEST_FORK_HEAP` (2500m),
`UNIT_TEST_RERUN_COUNT` (0), `RUN_CODECOVERAGE` (true).
- Shard rebalancing: `pinot-segment-local` tests moved to set #1 to keep
both shards near-equal in wall-clock.
- Per-fork JaCoCo aggregation via `jacoco-*.exec` glob.
## Backward compatibility
- No production code behavior changes. `ZkStarter`'s fork offset is inert
outside a surefire fork.
- All pom knobs default to the historical values (1 fork, 4g heap, 0 reruns,
single `jacoco.exec`), so `mvn test` locally behaves exactly as before.
## Testing
CI Unit Tests workflow (both shards) with parallel forks enabled.
--
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]
