This is an automated email from the ASF dual-hosted git repository. CRZbulabula pushed a commit to branch fix-remove-datanode-it-conflict-free-selector in repository https://gitbox.apache.org/repos/asf/iotdb.git
commit 641a14664862ec8d0e5a85e04bbc4eeb96013954 Author: Yongzao <[email protected]> AuthorDate: Wed Jul 1 16:25:49 2026 +0800 Make remove-DataNode IT conflict-free selection exhaustive to fix flakiness success1C5DRemoveTwoDataNodesUseSQL removes two DataNodes at once and picks them via selectRemoveDataNodesWithoutRegionConflict, which must avoid choosing two DataNodes that host replicas of the same consensus group. The previous selection was a single-pass greedy over a shuffled DataNode list: it unconditionally commits to the first shuffled DataNode and never backtracks. For k=2 this throws IllegalStateException whenever the first shuffled DataNode shares a consensus group with every other DataNode (a "hub"), even though a valid conflict-free pair exists among the others. In the 1C5D layout (5 data regions factor 2 + 1 schema region factor 3) such a hub commonly exists, so the test could abort with IllegalStateException before REMOVE DATANODE was even submitted. Replace the greedy with an exhaustive depth-first search with backtracking, so the selection fails only when no conflict-free set exists; the shuffle is kept so a random valid selection is still returned. Also correct a stale log message that said "timeout in 2 minutes" while awaitUntilSuccess waits 5 minutes. --- .../IoTDBRemoveDataNodeNormalIT.java | 2 +- .../removedatanode/IoTDBRemoveDataNodeUtils.java | 67 +++++++++++++++++----- 2 files changed, 55 insertions(+), 14 deletions(-) diff --git a/integration-test/src/test/java/org/apache/iotdb/confignode/it/removedatanode/IoTDBRemoveDataNodeNormalIT.java b/integration-test/src/test/java/org/apache/iotdb/confignode/it/removedatanode/IoTDBRemoveDataNodeNormalIT.java index 5eae82a35dd..bf0a6ce5527 100644 --- a/integration-test/src/test/java/org/apache/iotdb/confignode/it/removedatanode/IoTDBRemoveDataNodeNormalIT.java +++ b/integration-test/src/test/java/org/apache/iotdb/confignode/it/removedatanode/IoTDBRemoveDataNodeNormalIT.java @@ -357,7 +357,7 @@ public class IoTDBRemoveDataNodeNormalIT { removeSuccess = true; } catch (ConditionTimeoutException e) { if (expectRemoveSuccess) { - LOGGER.error("Remove DataNodes timeout in 2 minutes"); + LOGGER.error("Remove DataNodes timeout in 5 minutes"); Assert.fail(); } } diff --git a/integration-test/src/test/java/org/apache/iotdb/confignode/it/removedatanode/IoTDBRemoveDataNodeUtils.java b/integration-test/src/test/java/org/apache/iotdb/confignode/it/removedatanode/IoTDBRemoveDataNodeUtils.java index 2814faccc8f..2205554b1e9 100644 --- a/integration-test/src/test/java/org/apache/iotdb/confignode/it/removedatanode/IoTDBRemoveDataNodeUtils.java +++ b/integration-test/src/test/java/org/apache/iotdb/confignode/it/removedatanode/IoTDBRemoveDataNodeUtils.java @@ -37,6 +37,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -81,8 +82,9 @@ public class IoTDBRemoveDataNodeUtils { * generated {@code RemoveDataNodesProcedure} try to migrate two replicas of the same group at * once, which the ConfigNode rejects ("Only one replica of the same consensus group is allowed to * be migrated at the same time."). Randomly picking DataNodes (see {@link - * #selectRemoveDataNodes}) therefore makes multi-DataNode-remove tests flaky; this method keeps - * the selection valid and deterministic-enough for such tests. + * #selectRemoveDataNodes}) therefore makes multi-DataNode-remove tests flaky; this method instead + * searches exhaustively (with backtracking) for a conflict-free selection, so it fails only when + * no such selection exists rather than when an unlucky pick order dead-ends. * * @param allDataNodeId all registered DataNode ids * @param removeDataNodeNum how many DataNodes to remove @@ -106,20 +108,20 @@ public class IoTDBRemoveDataNodeUtils { .computeIfAbsent(dataNodeId, id -> new HashSet<>()) .add(regionId))); + // Shuffle so that, when several conflict-free selections exist, a random one is returned (keeps + // the coverage of this test varied across runs). List<Integer> shuffledDataNodeIds = new ArrayList<>(allDataNodeId); Collections.shuffle(shuffledDataNodeIds); - Set<Integer> selected = new HashSet<>(); - Set<Integer> coveredRegions = new HashSet<>(); - for (Integer dataNodeId : shuffledDataNodeIds) { - Set<Integer> regions = dataNodeToRegions.getOrDefault(dataNodeId, Collections.emptySet()); - if (Collections.disjoint(coveredRegions, regions)) { - selected.add(dataNodeId); - coveredRegions.addAll(regions); - if (selected.size() == removeDataNodeNum) { - return selected; - } - } + // Search exhaustively (with backtracking) for a set of DataNodes whose hosted region groups are + // pairwise disjoint. A single-pass greedy that unconditionally commits to the first shuffled + // DataNode can dead-end and wrongly throw even though a valid conflict-free selection exists - + // e.g. when that first DataNode happens to share a consensus group with every other DataNode - + // which made this test flaky. + Set<Integer> selected = new LinkedHashSet<>(); + if (searchConflictFreeDataNodes( + shuffledDataNodeIds, 0, removeDataNodeNum, new HashSet<>(), selected, dataNodeToRegions)) { + return selected; } throw new IllegalStateException( String.format( @@ -128,6 +130,45 @@ public class IoTDBRemoveDataNodeUtils { removeDataNodeNum, allDataNodeId, regionMap)); } + /** + * Depth-first search with backtracking for {@code need} DataNodes whose hosted region groups are + * pairwise disjoint. On success returns {@code true} and leaves the chosen ids in {@code + * selected}; on failure returns {@code false} with {@code selected} restored to its prior state. + * + * @param dataNodeIds candidate DataNode ids (iterated from {@code start} onwards) + * @param start index to start picking from, so each combination is visited at most once + * @param need how many DataNodes must be selected in total + * @param coveredRegions region groups already covered by the currently-selected DataNodes + * @param selected the DataNodes chosen so far (mutated during the search) + * @param dataNodeToRegions dataNodeId -> the region groups it hosts a replica of + */ + private static boolean searchConflictFreeDataNodes( + List<Integer> dataNodeIds, + int start, + int need, + Set<Integer> coveredRegions, + Set<Integer> selected, + Map<Integer, Set<Integer>> dataNodeToRegions) { + if (selected.size() == need) { + return true; + } + for (int i = start; i < dataNodeIds.size(); i++) { + Integer dataNodeId = dataNodeIds.get(i); + Set<Integer> regions = dataNodeToRegions.getOrDefault(dataNodeId, Collections.emptySet()); + if (Collections.disjoint(coveredRegions, regions)) { + selected.add(dataNodeId); + Set<Integer> nextCovered = new HashSet<>(coveredRegions); + nextCovered.addAll(regions); + if (searchConflictFreeDataNodes( + dataNodeIds, i + 1, need, nextCovered, selected, dataNodeToRegions)) { + return true; + } + selected.remove(dataNodeId); + } + } + return false; + } + public static void restartDataNodes(List<DataNodeWrapper> dataNodeWrappers) { dataNodeWrappers.parallelStream() .forEach(
