xiangfu0 opened a new pull request, #19272: URL: https://github.com/apache/pinot/pull/19272
## Summary Fixes two flaky unit tests that intermittently failed on CI (surfacing under parallel-fork load). Both are **root-cause synchronization/isolation fixes** — no `Thread.sleep`/retry masking. ## 1. `SegmentDeletionManagerTest.testRemoveDeletedSegments` (pinot-controller) **Symptom:** 120s timeout — `Failed to meet condition in 120000ms: dummyDir2 still exists`. **Root cause:** `SegmentDeletionManager.removeAgedDeletedSegments` deletes aged files **asynchronously** (on its single-threaded executor), but removes an emptied deleted-segments directory **synchronously, only on a subsequent run** (when it scans the dir and finds it already empty). The barrier before the second `removeAgedDeletedSegments()` call only waited for `dummyDir2.exists()` — trivially true from the moment the directory is created. Under load, the second run could race ahead of the first run's async file deletions, still observe files in `dummyDir2`, skip the empty-directory removal path, and never delete the directory. **Fix:** Strengthen the barrier to wait until `dummyDir2` **exists AND is empty** before triggering the next run, so the second run deterministically takes the empty-directory removal path. The `&&` short-circuit also keeps `list()` null-safe. ## 2. `DimensionTableDataManagerTest` (pinot-core) **Symptom:** intermittent `IllegalStateException: Failed to find schema for table: dimBaseballTeams_OFFLINE` in `testReloadTable`. **Root cause:** `DimensionTableDataManager` is a process-wide singleton keyed by table name (static `INSTANCES` map). Every test method loads the same `dimBaseballTeams_OFFLINE` table with **no per-method teardown**, so the singleton — with its property-store mock, loaded segments, and reload executor — leaked from one method into the next. A stale async reload from a prior method could read a `_propertyStore` that a concurrent `init()` had swapped out. **Fix:** Add an `@AfterMethod` that shuts the singleton down (removing it from `INSTANCES`) so each method starts from a clean, freshly-initialized instance. ## Testing Test-only changes. Pre-commit checks (spotless, checkstyle, license) pass on both modules. Rebased on current `master`. -- 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]
