CRZbulabula opened a new pull request, #18082:
URL: https://github.com/apache/iotdb/pull/18082

   ## Description
   
   `IoTDBRemoveDataNodeNormalIT.success1C5DRemoveTwoDataNodesUseSQL` (1C5D, 
schema replication factor 3, data replication factor 2, remove 2 DataNodes) has 
been failing intermittently on master even after #18075.
   
   ### Root cause
   
   The test removes two DataNodes in a single `REMOVE DATANODE a, b` statement 
and selects them with 
`IoTDBRemoveDataNodeUtils.selectRemoveDataNodesWithoutRegionConflict`. That 
selector must avoid picking two DataNodes that host replicas of the **same** 
consensus group, otherwise the generated `RemoveDataNodesProcedure` would try 
to migrate two replicas of one group at once, which the ConfigNode rejects 
(*"Only one replica of the same consensus group is allowed to be migrated at 
the same time."*). #18075 introduced this conflict-free selector to replace the 
previous random pick.
   
   However the selector was a **single-pass greedy over a shuffled DataNode 
list with no backtracking**: it always commits to the first shuffled DataNode 
and never reconsiders it. When removing 2 DataNodes it therefore throws 
`IllegalStateException` whenever the first shuffled DataNode shares a consensus 
group with **every** other DataNode (a "hub" in the region-sharing graph) — 
even though a valid conflict-free pair still exists among the other DataNodes.
   
   In the 1C5D layout the region-sharing graph (5 data-region groups at factor 
2, balanced to ~2 replicas per DataNode, plus one schema-region group at factor 
3) commonly contains such a hub DataNode, so a significant fraction of runs 
aborted with `IllegalStateException` **before** `REMOVE DATANODE` was ever 
submitted. This is a pure test-side flake, not a ConfigNode defect (the 
ConfigNode-side rejection path fixed by #18075 remains correct).
   
   ### Fix
   
   Replace the greedy with an **exhaustive depth-first search with 
backtracking** (`searchConflictFreeDataNodes`) that visits each combination at 
most once and fails only when no conflict-free set of the requested size 
actually exists. The initial `Collections.shuffle` is kept, so when several 
valid selections exist a random one is still returned (preserving the test's 
varied coverage across runs). The search space is tiny (a handful of DataNodes, 
k=2), so the cost is negligible, and this also future-proofs the helper for 
`removeDataNodeNum > 2`.
   
   Also corrects a stale log line that reported *"timeout in 2 minutes"* while 
`awaitUntilSuccess` actually waits 5 minutes.
   
   This is a **test-only** change; no production code is touched.
   
   <hr>
   
   This PR has:
   - [x] been self-reviewed.
   - [x] added comments explaining the "why" and the intent of the code.
   - [x] modified existing tests (integration-test helper) to remove flakiness.
   
   <hr>
   
   ##### Key changed/added classes (or packages if there are too many classes) 
in this PR
   - `IoTDBRemoveDataNodeUtils` (integration-test) — exhaustive backtracking 
conflict-free DataNode selection replacing the non-backtracking greedy.
   - `IoTDBRemoveDataNodeNormalIT` (integration-test) — corrected stale timeout 
log message.
   


-- 
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]

Reply via email to