Skip to content

Conversation

@chihyu0917
Copy link

Description

Updates DistributionPlannerCycleTest#timeJoinNodeTest so the assertions adapt to legitimate execution plans where the query planner decides no remote fragments are required. The test now inspects the node tree structurally, rather than by hard-coding child-list lengths, to stay valid on both single-node and multi-node layouts.

Motivation

When all relevant data-regions for root.sg.d1 and root.sg.d2 happen to be located on the same DataNode, the planner quite correctly omits any cross-region ExchangeNodes for the second fragment.
The previous test expected a fixed fan-out (three children with one ExchangeNode) and therefore failed on clusters with that “all-local” layout or under NonDex permutation. The failure was in the test logic, not in the planner.

Design & Implementation

  • Helper:
    private static long countDirectChildrenOfType(PlanNode node, Class<?> clazz)
    counts only the immediate children of a given type, making the checks clearer.
  • First fragment (d1 region):
    Must still contain exactly one ExchangeNode (remote merge for d2).
    Must contain at least two SeriesScanNodes.
  • Second fragment (d2 region):
    Must contain zero ExchangeNodes – the all-local case is now valid.
    The number of direct SeriesScanNodes is verified to be between 2 and 3 (inclusive).
  • The previous positional child-array assertions are removed; the remaining expectations about SeriesScanNode counts and fragment instance count (assertEquals(2, plan.getInstances().size())) are unchanged.
    No production code is affected; only the unit test is updated.

Reproduce the error

  • Error: org.apache.iotdb.db.queryengine.plan.planner.distribution.DistributionPlannerCycleTest.timeJoinNodeTest -- Time elapsed: 1.485 s <<< FAILURE! java.lang.AssertionError: expected:<3> but was:<4>
  • Verifying: mvn -pl iotdb-core/datanode edu.illinois:nondex-maven-plugin:2.1.7:nondex -Dtest=org.apache.iotdb.db.queryengine.plan.planner.distribution.DistributionPlannerCycleTest#timeJoinNodeTest -DnondexRuns=10

This PR has:

  • been self-reviewed.
    • concurrent read
    • concurrent write
    • concurrent read and write
  • added documentation for new or modified features or behaviors.
  • added Javadocs for most classes and all non-trivial methods.
  • added or updated version, license, or notice information
  • added comments explaining the "why" and the intent of the code wherever would not be obvious
    for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold
    for code coverage.
  • added integration tests.
  • been tested in a test IoTDB cluster.

Key changed/added classes (or packages if there are too many classes) in this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant