This is an automated email from the ASF dual-hosted git repository.
xiangfu0 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 4efaa151d78 Fix two flaky unit tests exposed under parallel-fork CI
load (#19272)
4efaa151d78 is described below
commit 4efaa151d78e97247400ad14c1b5ee2e4741be8c
Author: Xiang Fu <[email protected]>
AuthorDate: Mon Aug 17 12:17:46 2026 -0700
Fix two flaky unit tests exposed under parallel-fork CI load (#19272)
SegmentDeletionManagerTest.testRemoveDeletedSegments:
Aged files are deleted asynchronously on SegmentDeletionManager's executor,
while an empty deleted-segments directory is only removed synchronously on a
subsequent run. The barrier before the second removeAgedDeletedSegments()
call
only waited for dummyDir2.exists(), which is trivially true from directory
creation. 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, and never delete the directory -- surfacing as a 120s "dummyDir2
still
exists" timeout. Strengthen the barrier to wait until dummyDir2 exists AND
is
empty before triggering the next run.
DimensionTableDataManagerTest:
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 across methods. A stale
async reload from a prior method could read a _propertyStore that a
concurrent
init() had swapped out, intermittently failing testReloadTable with "Failed
to
find schema for table: dimBaseballTeams_OFFLINE". Add an @AfterMethod that
shuts
the singleton down (removing it from INSTANCES) so each method starts from a
clean, freshly-initialized instance.
Both are root-cause synchronization/isolation fixes, not sleep/retry masks.
---
.../helix/core/util/SegmentDeletionManagerTest.java | 15 +++++++++++----
.../manager/offline/DimensionTableDataManagerTest.java | 18 ++++++++++++++++++
2 files changed, 29 insertions(+), 4 deletions(-)
diff --git
a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/util/SegmentDeletionManagerTest.java
b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/util/SegmentDeletionManagerTest.java
index bf36f7a8660..10d8c6beb5e 100644
---
a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/util/SegmentDeletionManagerTest.java
+++
b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/util/SegmentDeletionManagerTest.java
@@ -300,15 +300,22 @@ public class SegmentDeletionManagerTest {
TestUtils.waitForCondition((aVoid) -> dummyDir1.list().length == 1, 2000,
120000,
"Unable to delete desired segments from dummyDir1");
- // Check that empty directory has not been removed in the first run
- TestUtils.waitForCondition((aVoid) -> dummyDir2.exists(), 2000, 120000,
- "dummyDir2 does not exist");
+ // dummyDir2's files are all aged, so the first run deletes them
(asynchronously, on the deletion
+ // manager's executor) but leaves the now-empty directory in place. Empty
directories are only
+ // removed on a subsequent run. Wait until the directory is present AND
empty before triggering
+ // that next run: this barrier both asserts the "directory survives the
first run" behavior and,
+ // critically, guarantees the async file deletions have completed.
Previously this only waited for
+ // dummyDir2.exists() (trivially true from the moment it is created), so
the second run below could
+ // race ahead of the async deletions, still see files in dummyDir2, skip
the empty-directory
+ // removal, and never delete the directory -- surfacing as a 120s
"dummyDir2 still exists" timeout.
+ TestUtils.waitForCondition((aVoid) -> dummyDir2.exists() &&
dummyDir2.list().length == 0, 2000, 120000,
+ "dummyDir2 was not emptied by the first deletion run");
// Check that deleted file without retention suffix is honoring
cluster-wide retention period of 7 days.
TestUtils.waitForCondition((aVoid) -> dummyDir3.list().length == 1, 2000,
120000,
"Unable to delete desired segments from dummyDir3");
- // Try to remove empty directory in the next run
+ // Try to remove the now-empty directory in the next run
deletionManager.removeAgedDeletedSegments(leadControllerManager);
TestUtils.waitForCondition((aVoid) -> !dummyDir2.exists(), 2000, 120000,
"dummyDir2 still exists");
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/data/manager/offline/DimensionTableDataManagerTest.java
b/pinot-core/src/test/java/org/apache/pinot/core/data/manager/offline/DimensionTableDataManagerTest.java
index 2bae01b1e41..57bbc31018f 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/data/manager/offline/DimensionTableDataManagerTest.java
+++
b/pinot-core/src/test/java/org/apache/pinot/core/data/manager/offline/DimensionTableDataManagerTest.java
@@ -65,6 +65,7 @@ import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
import org.apache.pinot.spi.utils.builder.TableNameBuilder;
import org.testng.Assert;
import org.testng.annotations.AfterClass;
+import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
@@ -124,6 +125,23 @@ public class DimensionTableDataManagerTest {
_segmentZKMetadata.setCrc(Long.parseLong(segmentMetadata.getCrc()));
}
+ @AfterMethod(alwaysRun = true)
+ public void tearDownMethod() {
+ // DimensionTableDataManager is a process-wide singleton keyed by table
name (see the static
+ // INSTANCES map in DimensionTableDataManager). Every test method loads
the same
+ // dimBaseballTeams_OFFLINE table, so without an explicit teardown the
singleton (and its
+ // property-store mock, loaded segments, and reload executor) leaks from
one method into the
+ // next. A stale async reload from a prior method could then read a
_propertyStore that a
+ // concurrent init() had swapped out, surfacing as an intermittent
+ // "Failed to find schema for table: dimBaseballTeams_OFFLINE". Shutting
the singleton down
+ // removes it from INSTANCES so each method starts from a clean,
freshly-initialized instance.
+ DimensionTableDataManager tableDataManager =
+ DimensionTableDataManager.getInstanceByTableName(OFFLINE_TABLE_NAME);
+ if (tableDataManager != null) {
+ tableDataManager.shutDown();
+ }
+ }
+
@AfterClass
public void tearDown() {
FileUtils.deleteQuietly(TEMP_DIR);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
