iotdb icon indicating copy to clipboard operation
iotdb copied to clipboard

fix testTimeJoinAggregationSinglePerRegion

Open kabo87777 opened this issue 3 months ago • 0 comments

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

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:

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):

List<FragmentInstance> fragmentInstances = plan.getInstances();
List<AggregationStep> expected = Arrays.asList(AggregationStep.STATIC, AggregationStep.FINAL);
verifyAggregationStep(expected, fragmentInstances.get(0).getFragment().getPlanNodeTree());

After (Order-Independent):

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):

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

kabo87777 avatar Oct 19 '25 15:10 kabo87777