kabo87777 opened a new pull request, #16616:
URL: https://github.com/apache/iotdb/pull/16616
# Fix Non-Deterministic Behavior in
AggregationDistributionTest.testTimeJoinAggregationSinglePerRegion
## Problem
The test `testTimeJoinAggregationSinglePerRegion` was failing
non-deterministically under NonDex with a 40% failure rate (2 out of 5 runs)
due to order-dependent fragment access.
## Way to Reproduce
```bash
cd iotdb-core/datanode
mvn edu.illinois:nondex-maven-plugin:2.1.1:nondex \
-Dtest=AggregationDistributionTest#testTimeJoinAggregationSinglePerRegion \
-DnondexRuns=5
# Expected: Test fails with certain NonDex seeds
# Failure: AssertionError when fragment 0 doesn't have expected aggregation
steps
```
## Root Cause
The test assumed fragment at index 0 always contains the `STATIC` and
`FINAL` aggregation steps:
```java
List<FragmentInstance> fragmentInstances = plan.getInstances();
List<AggregationStep> expected = Arrays.asList(AggregationStep.STATIC,
AggregationStep.FINAL);
verifyAggregationStep(expected,
fragmentInstances.get(0).getFragment().getPlanNodeTree());
```
When NonDex shuffled collection iteration order during distributed query
planning, the fragment containing these specific aggregation steps appeared at
different positions in the list, causing the test to fail even though the query
plan was semantically correct.
## Solution
Made the test **order-independent** by searching all fragments for the
expected aggregation steps instead of assuming a fixed position:
**Before (Order-Dependent):**
```java
List<FragmentInstance> fragmentInstances = plan.getInstances();
List<AggregationStep> expected = Arrays.asList(AggregationStep.STATIC,
AggregationStep.FINAL);
verifyAggregationStep(expected,
fragmentInstances.get(0).getFragment().getPlanNodeTree());
```
**After (Order-Independent):**
```java
List<FragmentInstance> fragmentInstances = plan.getInstances();
List<AggregationStep> expected = Arrays.asList(AggregationStep.STATIC,
AggregationStep.FINAL);
boolean foundExpectedFragment = false;
for (FragmentInstance instance : fragmentInstances) {
try {
verifyAggregationStep(expected,
instance.getFragment().getPlanNodeTree());
foundExpectedFragment = true;
break;
} catch (AssertionError e) {
// This fragment doesn't have the expected steps, continue checking
others
}
}
assertTrue(
"Expected at least one fragment with STATIC and FINAL aggregation steps",
foundExpectedFragment);
```
### Key Improvements:
1. **Iterates through all fragments**: No longer assumes the target fragment
is at position 0
2. **Try-catch pattern matching**: Attempts verification on each fragment
until finding a match
3. **Early exit**: Breaks loop once expected fragment is found for efficiency
4. **Clear assertion message**: Provides helpful error message if no
fragment matches
## Verification
Tested with 50 NonDex runs - **0 failures** (100% success rate):
```bash
mvn edu.illinois:nondex-maven-plugin:2.1.1:nondex \
-Dtest=AggregationDistributionTest#testTimeJoinAggregationSinglePerRegion \
-DnondexRuns=50
# Result: 0 failures
```
## Key Changed Classes
- **AggregationDistributionTest**:
- Modified `testTimeJoinAggregationSinglePerRegion` to search all
fragments for expected aggregation steps
- Changed from fixed index access to iterative search with pattern matching
--
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]
