Skip to content

[Test] Fix dynamic status queries and asynchronous deletion polling - #18899

Open
Caideyipi wants to merge 6 commits into
apache:masterfrom
Caideyipi:fix/table-drop-progress-single-node
Open

Caideyipi wants to merge 6 commits into
apache:masterfrom
Caideyipi:fix/table-drop-progress-single-node

Conversation

@Caideyipi

@Caideyipi Caideyipi commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Cluster ITs can report InconsistentDataException when information_schema.drop_table_procedures changes from RUNNABLE to SUCCESS between queries to different DataNodes. DROP TABLE also acknowledges submission before deletion finishes, which can race with missing-table assertions, database cleanup, and historical pipe creation.

  • Read procedure progress and node-local pipe memory diagnostics through a single DataNode. Share the latest-procedure wait across the table management, deletion, and DDL HA tests; fail promptly on rollback. Ordinary data queries retain cross-node verification.
  • Retry database cleanup only for OVERLAP_WITH_EXISTING_TASK in the deletion, insert-query, CTE, explain-analyze, and pipe metadata tests. Wait for table deletion before starting the historical transfer in IoTDBPipeWithLoadIT.
  • Fetch a fresh ConfigNode snapshot on every pipe auto-drop poll in both data models. Previously, the tests reused an exhausted SHOW PIPES result set, which could falsely indicate successful deletion. Table pipe checks explicitly select the table model.
  • Restore the continuous assertions in TestUtils: assertAlwaysFail and all three polling overloads of assertDataAlwaysOnEnv previously configured failFast without a terminal await operation, so no queries ran. Execute checks throughout the requested observation period, select the correct SQL dialect, and compare flush timing in milliseconds. Correct the dormant callers' result headers, SQL dialects, and current privilege names; wait for permission propagation where the expected result is asynchronous. Requested observation periods, including explicit 600-second calls, now actually run.
  • Add IoTDBTestUtilsIT regression coverage for observation duration, rejected result mismatches, failure callbacks, and unexpected query success.

The DROP TABLE call sites were reviewed against origin/master (9a078cc105e), including JDBC, relational session, pipe, audit-log, and C++ ITs. Same-name CREATE TABLE already waits for overlapping DDL on the server; those paths do not need additional client polling. The relational session cleanup retry is already present upstream.

Validation

  • Spotless, integration-test compilation, Checkstyle, and license checks passed.

  • 52 distinct targeted IT methods passed with no remaining failures or errors:

    Profile Methods Coverage
    TableClusterIT 28 Table management and progress, deletion, insert-query cleanup, CTE cleanup, database diagnostics, both DDL HA scenarios, and the three helper regression tests
    TableSimpleIT 3 Explain-analyze cleanup
    MultiClusterIT 21 Tree/table pipe auto-drop, historical load after deletion, metadata, permissions, schema-only inclusion, and authentication propagation

The counts exclude repeat runs of the same method. C++ call sites were reviewed statically; the full IT suite was not run.

The new helper regression tests run with TableClusterIT (1 ConfigNode and 3 DataNodes):

mvn -nsu verify -pl integration-test -P with-integration-tests,TableClusterIT \
  -DskipUTs=true -Dit.test=IoTDBTestUtilsIT -Ddevelocity.off=true

Local runs constrained the test-driver heap to 256-512 MiB. Final Pipe runs also used -XX:ActiveProcessorCount=2, Serial GC, and a 32 MiB initial node heap to fit the machine's memory limits; DataNode maximum heap and direct-memory limits remained at 256 MiB. Cluster topologies and assertion durations were preserved.

  • Self-reviewed.
  • Documented the helpers and asynchronous waits.
  • Added integration regression tests.
  • Tested the affected cases in IoTDB clusters.

@Caideyipi Caideyipi changed the title [Test] Query drop table procedure progress on a single DataNode [Test] Fix dynamic status queries and asynchronous deletion polling Oct 16, 2026

@JackieTien97 JackieTien97 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes for two confirmed issues: enabling continuous assertions makes an existing lifecycle test fail, and the new auto-drop polling can interpret an RPC error as successful deletion. Reproduction details and suggested fixes are inline. The three new IoTDBTestUtilsIT regression tests passed.

Comment on lines +2107 to +2108
})
.until(() -> true);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Fix the remaining lifecycle expectations when enabling continuous assertions

This terminal await also activates the previously dormant checks in IoTDBPipeLifeCycleIT.testLifeCycleWithHistoryDisabled, but that caller has not been updated. After stopPipe("p1"), root.ln0 is created only on the sender, while the receiver retains the two existing series under root.db. The receiver's series count under root.ln0, series count under root.db, and database count for root.ln0 should therefore be 0, 2, and 0; the assertions at lines 227–238 still expect 1 for all three.

I reproduced the failure from a clean reactor build with MultiClusterIT and two 1-ConfigNode/3-DataNode clusters: the first incorrect assertion fails at IoTDBPipeLifeCycleIT.java:227, with actual 0 versus expected 1. Please update all three expectations together with this helper change so the existing tree-model pipe test remains runnable.

@Caideyipi Caideyipi Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied in f15d245. The lifecycle assertions now expect 0 series under root.ln0, 2 under root.db, and 0 databases under root.ln0. Verified testLifeCycleWithHistoryDisabled with two 1-ConfigNode/3-DataNode clusters and the original observation periods; it passed. The local run used 32 MiB initial node heaps, Serial GC, and ActiveProcessorCount=2 to fit available memory; maximum node heaps and cluster topology were unchanged.

Comment on lines +139 to +144
final List<TShowPipeInfo> showPipeResult =
client.showPipe(
new TShowPipeReq()
.setIsTableModel(true)
.setUserName(SessionConfig.DEFAULT_USER))
.pipeInfoList;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Check the ShowPipe response status before accepting an empty list

The direct RPC does not translate a non-success TSStatus into an exception as the JDBC query did. In PipeTaskCoordinator.showPipes(), the ConsensusException handler returns EXECUTE_STATEMENT_ERROR together with an empty pipeInfoList. Reading only the list makes both noneMatch(...) and isEmpty() accept that failed read as successful pipe deletion, even though deletion has not been established.

I verified this using the production error-response conversion: a response with status 301 passes both removal predicates. Please assert that the response status is TSStatusCode.SUCCESS_STATUS before examining the list, in all four new polling blocks across the table-model and tree-model auto-drop tests.

@Caideyipi Caideyipi Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied in f15d245 and 3b565d1. All four newly introduced auto-drop polling blocks now assert SUCCESS_STATUS before examining pipeInfoList. The same check also covers the two existing direct-RPC polls in the tree-model test, so all six polls in these two classes validate status first. All six IoTDBPipeAutoDropIT methods passed with MultiClusterIT (two 1-ConfigNode/3-DataNode clusters per test). Local runs used 32 MiB initial node heaps, Serial GC, and ActiveProcessorCount=2; maximum node heaps and cluster topology were unchanged.

@hongzhi-gao

Copy link
Copy Markdown
Contributor

Automated preliminary correctness review (Codex CLI, based on PR head f15d24542935). No tests were run.

  1. integration-test/src/test/java/org/apache/iotdb/db/it/utils/TestUtils.java:2088
    • Trigger: IoTDBPipeMetaIT.testAttributeSync reaches assertDataAlwaysOnEnv before the newly created pipe has transferred the sender's attributes.
    • Potential issue: The restored fail-fast assertion queries immediately and rejects the first mismatch. testAttributeSync initializes receiver attributes as (1,1,null) and (2,2,null), creates the pipe on the sender, then immediately expects merged attributes (1,1,1) and (2,2,2). There is no receiver convergence wait. A valid asynchronous transfer can therefore fail this test before replication completes. Wait for the merged attributes with assertDataEventuallyOnEnv before asserting their continued stability.

This review is AI-assisted and not an approval or request for changes. Please verify each scenario before acting.

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.

3 participants