Skip to content

Commit f5175b9

Browse files
CRZbulabulaCRZbulabula
authored
Fix flaky region-migration/cluster ITs and enable IoTV2 daily migration tests (#17924)
1 parent 71e9c59 commit f5175b9

9 files changed

Lines changed: 145 additions & 59 deletions

‎integration-test/src/test/java/org/apache/iotdb/confignode/it/regionmigration/IoTDBRegionOperationReliabilityITFramework.java‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,17 @@ private void generalTestWithAllOptions(
355355
Assert.fail();
356356
}
357357

358+
// The kill point is detected by a background thread tailing the node log, so the migration
359+
// result (observed by awaitUntilSuccess above) can become visible before that thread has read
360+
// and processed the kill-point line of the last migration phase (e.g.
361+
// RemoveRegionLocationCache). Give that thread a short grace period to catch up before
362+
// asserting, otherwise checkKillPointsAllTriggered may fail spuriously with "Some kill points
363+
// was not triggered". This is best-effort: the authoritative assertion remains
364+
// checkKillPointsAllTriggered, which still fails the test if a kill point genuinely never
365+
// triggers (e.g. the badKillPoint test).
366+
graceWaitForKillPointsTriggered(configNodeKeywords);
367+
graceWaitForKillPointsTriggered(dataNodeKeywords);
368+
358369
// make sure all kill points have been triggered
359370
checkKillPointsAllTriggered(configNodeKeywords);
360371
checkKillPointsAllTriggered(dataNodeKeywords);
@@ -520,6 +531,26 @@ private static void awaitKillPointsTriggered(KeySetView<String, Boolean> killPoi
520531
Awaitility.await().atMost(2, TimeUnit.MINUTES).until(killPoints::isEmpty);
521532
}
522533

534+
/**
535+
* Best-effort wait for all kill points to be triggered. The kill point is detected by a
536+
* background thread tailing the node log, so there can be a short lag between the migration
537+
* result becoming visible and that thread processing the kill-point line. This gives it a brief
538+
* grace period to catch up. Unlike {@link #awaitKillPointsTriggered}, it never throws: the
539+
* authoritative check is {@link #checkKillPointsAllTriggered}, so a kill point that genuinely
540+
* never triggers (e.g. the badKillPoint test) is still caught there as an AssertionError rather
541+
* than masked here.
542+
*/
543+
private static void graceWaitForKillPointsTriggered(KeySetView<String, Boolean> killPoints) {
544+
if (killPoints.isEmpty()) {
545+
return;
546+
}
547+
try {
548+
Awaitility.await().atMost(1, TimeUnit.MINUTES).until(killPoints::isEmpty);
549+
} catch (ConditionTimeoutException ignored) {
550+
// Fall through to checkKillPointsAllTriggered, which makes the real assertion.
551+
}
552+
}
553+
523554
private static String buildRegionMigrateCommand(int who, int from, int to) {
524555
String result = String.format(REGION_MIGRATE_COMMAND_FORMAT, who, from, to);
525556
LOGGER.info(result);

‎integration-test/src/test/java/org/apache/iotdb/confignode/it/regionmigration/pass/daily/datanodecrash/iotv2/batch/IoTDBRegionMigrateDataNodeCrashForIoTV2BatchIT.java‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,10 @@
2525
import org.apache.iotdb.it.framework.IoTDBTestRunner;
2626
import org.apache.iotdb.itbase.category.DailyIT;
2727

28+
import org.junit.Test;
2829
import org.junit.experimental.categories.Category;
2930
import org.junit.runner.RunWith;
3031

31-
// TODO: @Yongzao Dan, reopen this CI after discussion with @HxpSerein
32-
3332
@Category({DailyIT.class})
3433
@RunWith(IoTDBTestRunner.class)
3534
public class IoTDBRegionMigrateDataNodeCrashForIoTV2BatchIT
@@ -41,7 +40,7 @@ public class IoTDBRegionMigrateDataNodeCrashForIoTV2BatchIT
4140
private final int configNodeNum = 1;
4241
private final int dataNodeNum = 3;
4342

44-
// @Test
43+
@Test
4544
public void coordinatorCrashDuringAddPeerTransition() throws Exception {
4645
failTest(
4746
2,
@@ -53,7 +52,7 @@ public void coordinatorCrashDuringAddPeerTransition() throws Exception {
5352
KillNode.COORDINATOR_DATANODE);
5453
}
5554

56-
// @Test
55+
@Test
5756
public void coordinatorCrashDuringAddPeerDone() throws Exception {
5857
failTest(
5958
2,
@@ -69,9 +68,13 @@ public void coordinatorCrashDuringAddPeerDone() throws Exception {
6968

7069
// region Original DataNode crash tests
7170

72-
// @Test
71+
@Test
7372
public void originalCrashDuringAddPeerDone() throws Exception {
74-
failTest(
73+
// Once the add-peer phase is done, the new peer already holds the data, so the migration is
74+
// designed to tolerate the original (source) DataNode crashing afterwards: it completes
75+
// successfully and merely leaves the region files on the dead node to be cleaned up later.
76+
// Hence this is a successTest, not a failTest.
77+
successTest(
7578
2,
7679
2,
7780
1,
@@ -85,7 +88,7 @@ public void originalCrashDuringAddPeerDone() throws Exception {
8588

8689
// region Destination DataNode crash tests
8790

88-
// @Test
91+
@Test
8992
public void destinationCrashDuringCreateLocalPeer() throws Exception {
9093
failTest(
9194
2,
@@ -97,7 +100,7 @@ public void destinationCrashDuringCreateLocalPeer() throws Exception {
97100
KillNode.DESTINATION_DATANODE);
98101
}
99102

100-
// @Test
103+
@Test
101104
public void destinationCrashDuringAddPeerDone() throws Exception {
102105
failTest(
103106
2,

‎integration-test/src/test/java/org/apache/iotdb/confignode/it/regionmigration/pass/daily/datanodecrash/iotv2/stream/IoTDBRegionMigrateDataNodeCrashForIoTV2StreamIT.java‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,10 @@
2828
import org.apache.iotdb.itbase.category.DailyIT;
2929

3030
import org.junit.Before;
31+
import org.junit.Test;
3132
import org.junit.experimental.categories.Category;
3233
import org.junit.runner.RunWith;
3334

34-
// TODO: @Yongzao Dan, reopen this CI after discussion with @HxpSerein
35-
3635
@Category({DailyIT.class})
3736
@RunWith(IoTDBTestRunner.class)
3837
public class IoTDBRegionMigrateDataNodeCrashForIoTV2StreamIT
@@ -54,7 +53,7 @@ public void setUp() throws Exception {
5453
.setIoTConsensusV2Mode(ConsensusFactory.IOT_CONSENSUS_V2_STREAM_MODE);
5554
}
5655

57-
// @Test
56+
@Test
5857
public void coordinatorCrashDuringAddPeerTransition() throws Exception {
5958
failTest(
6059
2,
@@ -66,7 +65,7 @@ public void coordinatorCrashDuringAddPeerTransition() throws Exception {
6665
KillNode.COORDINATOR_DATANODE);
6766
}
6867

69-
// @Test
68+
@Test
7069
public void coordinatorCrashDuringAddPeerDone() throws Exception {
7170
failTest(
7271
2,
@@ -82,9 +81,13 @@ public void coordinatorCrashDuringAddPeerDone() throws Exception {
8281

8382
// region Original DataNode crash tests
8483

85-
// @Test
84+
@Test
8685
public void originalCrashDuringAddPeerDone() throws Exception {
87-
failTest(
86+
// Once the add-peer phase is done, the new peer already holds the data, so the migration is
87+
// designed to tolerate the original (source) DataNode crashing afterwards: it completes
88+
// successfully and merely leaves the region files on the dead node to be cleaned up later.
89+
// Hence this is a successTest, not a failTest.
90+
successTest(
8891
2,
8992
2,
9093
1,
@@ -98,7 +101,7 @@ public void originalCrashDuringAddPeerDone() throws Exception {
98101

99102
// region Destination DataNode crash tests
100103

101-
// @Test
104+
@Test
102105
public void destinationCrashDuringCreateLocalPeer() throws Exception {
103106
failTest(
104107
2,
@@ -110,7 +113,7 @@ public void destinationCrashDuringCreateLocalPeer() throws Exception {
110113
KillNode.DESTINATION_DATANODE);
111114
}
112115

113-
// @Test
116+
@Test
114117
public void destinationCrashDuringAddPeerDone() throws Exception {
115118
failTest(
116119
2,

‎integration-test/src/test/java/org/apache/iotdb/confignode/it/regionmigration/pass/daily/iotv2/batch/IoTDBRegionMigrateClusterCrashIoTV2BatchIT.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,7 @@ public void clusterCrash1() throws Exception {
3939
killClusterTest(buildSet(AddRegionPeerState.CREATE_NEW_REGION_PEER), true);
4040
}
4141

42-
// TODO: @Yongzao Dan, reopen this CI after discussion with @HxpSerein
43-
// @Test
42+
@Test
4443
public void clusterCrash2() throws Exception {
4544
killClusterTest(buildSet(AddRegionPeerState.DO_ADD_REGION_PEER), false);
4645
}
@@ -60,8 +59,7 @@ public void clusterCrash6() throws Exception {
6059
killClusterTest(buildSet(RemoveRegionPeerState.REMOVE_REGION_PEER), true);
6160
}
6261

63-
// TODO: @Yongzao Dan, reopen this CI after discussion with @HxpSerein
64-
// @Test
62+
@Test
6563
public void clusterCrash7() throws Exception {
6664
killClusterTest(buildSet(RemoveRegionPeerState.DELETE_OLD_REGION_PEER), true);
6765
}

‎integration-test/src/test/java/org/apache/iotdb/confignode/it/regionmigration/pass/daily/iotv2/batch/IoTDBRegionMigrateConfigNodeCrashIoTV2BatchIT.java‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,7 @@ public void cnCrashDuringCreatePeerTest() throws Exception {
6666
KillNode.CONFIG_NODE);
6767
}
6868

69-
// TODO: @Yongzao Dan, reopen this CI after discussion with @HxpSerein
70-
// @Test
69+
@Test
7170
public void testCnCrashDuringDoAddPeer() throws Exception {
7271
successTest(
7372
1,
@@ -127,8 +126,7 @@ public void cnCrashDuringDeleteOldRegionPeerTest() throws Exception {
127126
KillNode.CONFIG_NODE);
128127
}
129128

130-
// TODO: @Yongzao Dan, reopen this CI after discussion with @HxpSerein
131-
// @Test
129+
@Test
132130
public void cnCrashDuringRemoveRegionLocationCacheTest() throws Exception {
133131
successTest(
134132
1,
@@ -140,8 +138,7 @@ public void cnCrashDuringRemoveRegionLocationCacheTest() throws Exception {
140138
KillNode.CONFIG_NODE);
141139
}
142140

143-
// TODO: @Yongzao Dan, reopen this CI after discussion with @HxpSerein
144-
// @Test
141+
@Test
145142
public void cnCrashTest() throws Exception {
146143
ConcurrentHashMap.KeySetView<String, Boolean> killConfigNodeKeywords = noKillPoints();
147144
killConfigNodeKeywords.addAll(

‎integration-test/src/test/java/org/apache/iotdb/confignode/it/regionmigration/pass/daily/iotv2/stream/IoTDBRegionMigrateClusterCrashIoTV2StreamIT.java‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,12 @@ public void setUp() throws Exception {
4747
.setIoTConsensusV2Mode(ConsensusFactory.IOT_CONSENSUS_V2_STREAM_MODE);
4848
}
4949

50-
// TODO: @Yongzao Dan, reopen this CI after discussion with @HxpSerein
51-
// @Test
50+
@Test
5251
public void clusterCrash1() throws Exception {
5352
killClusterTest(buildSet(AddRegionPeerState.CREATE_NEW_REGION_PEER), true);
5453
}
5554

56-
// TODO: @Yongzao Dan, reopen this CI after discussion with @HxpSerein
57-
// @Test
55+
@Test
5856
public void clusterCrash2() throws Exception {
5957
killClusterTest(buildSet(AddRegionPeerState.DO_ADD_REGION_PEER), false);
6058
}
@@ -74,8 +72,7 @@ public void clusterCrash6() throws Exception {
7472
killClusterTest(buildSet(RemoveRegionPeerState.REMOVE_REGION_PEER), true);
7573
}
7674

77-
// TODO: @Yongzao Dan, reopen this CI after discussion with @HxpSerein
78-
// @Test
75+
@Test
7976
public void clusterCrash7() throws Exception {
8077
killClusterTest(buildSet(RemoveRegionPeerState.DELETE_OLD_REGION_PEER), true);
8178
}

‎integration-test/src/test/java/org/apache/iotdb/confignode/it/regionmigration/pass/daily/iotv2/stream/IoTDBRegionMigrateConfigNodeCrashIoTV2StreamIT.java‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,7 @@ public void cnCrashDuringCreatePeerTest() throws Exception {
8080
KillNode.CONFIG_NODE);
8181
}
8282

83-
// TODO: @Yongzao Dan, reopen this CI after discussion with @HxpSerein
84-
// @Test
83+
@Test
8584
public void testCnCrashDuringDoAddPeer() throws Exception {
8685
successTest(
8786
1,
@@ -141,8 +140,7 @@ public void cnCrashDuringDeleteOldRegionPeerTest() throws Exception {
141140
KillNode.CONFIG_NODE);
142141
}
143142

144-
// TODO: @Yongzao Dan, reopen this CI after discussion with @HxpSerein
145-
// @Test
143+
@Test
146144
public void cnCrashDuringRemoveRegionLocationCacheTest() throws Exception {
147145
successTest(
148146
1,
@@ -154,8 +152,7 @@ public void cnCrashDuringRemoveRegionLocationCacheTest() throws Exception {
154152
KillNode.CONFIG_NODE);
155153
}
156154

157-
// TODO: @Yongzao Dan, reopen this CI after discussion with @HxpSerein
158-
// @Test
155+
@Test
159156
public void cnCrashTest() throws Exception {
160157
ConcurrentHashMap.KeySetView<String, Boolean> killConfigNodeKeywords = noKillPoints();
161158
killConfigNodeKeywords.addAll(

‎integration-test/src/test/java/org/apache/iotdb/db/it/IoTDBCustomizedClusterIT.java‎

Lines changed: 38 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,13 @@
2525
import org.apache.iotdb.it.env.cluster.node.DataNodeWrapper;
2626
import org.apache.iotdb.it.framework.IoTDBTestRunner;
2727
import org.apache.iotdb.itbase.category.DailyIT;
28+
import org.apache.iotdb.itbase.exception.InconsistentDataException;
2829
import org.apache.iotdb.rpc.IoTDBConnectionException;
2930
import org.apache.iotdb.rpc.StatementExecutionException;
3031
import org.apache.iotdb.rpc.TSStatusCode;
3132
import org.apache.iotdb.session.Session;
3233

34+
import org.awaitility.Awaitility;
3335
import org.junit.Test;
3436
import org.junit.experimental.categories.Category;
3537
import org.junit.runner.RunWith;
@@ -64,22 +66,42 @@ public void testRepeatedlyRestartWholeClusterWithWrite() throws Exception {
6466
testRepeatedlyRestartWholeCluster(
6567
(s, i, env) -> {
6668
if (i != 0) {
67-
ResultSet resultSet = s.executeQuery("SELECT last s1 FROM root.**");
68-
ResultSetMetaData metaData = resultSet.getMetaData();
69-
assertEquals(4, metaData.getColumnCount());
70-
int cnt = 0;
71-
while (resultSet.next()) {
72-
cnt++;
73-
StringBuilder result = new StringBuilder();
74-
for (int j = 0; j < metaData.getColumnCount(); j++) {
75-
result
76-
.append(metaData.getColumnName(j + 1))
77-
.append(":")
78-
.append(resultSet.getString(j + 1))
79-
.append(",");
80-
}
81-
System.out.println(result);
82-
}
69+
// This query is fanned out to every DataNode and the results are compared across
70+
// replicas. Right after a restart the last cache on each coordinator is reloaded
71+
// lazily, so the cross-replica comparison may transiently observe an inconsistent
72+
// result until the cluster converges. ORDER BY TIMESERIES makes the row order
73+
// deterministic across coordinators (the root cause of the observed flakiness), and the
74+
// retry tolerates the brief convergence window (e.g. a replica that has not finished
75+
// recovering yet) without masking a genuine, persistent inconsistency.
76+
//
77+
// ignoreExceptionsMatching(InconsistentDataException) is required: a mismatch surfaces
78+
// as InconsistentDataException (a RuntimeException) thrown from getString(), and
79+
// untilAsserted() only retries on AssertionError by default, so without it the retry
80+
// would not actually cover this failure. We match only InconsistentDataException so a
81+
// genuine error (e.g. a real SQLException) still fails fast instead of being retried.
82+
Awaitility.await()
83+
.atMost(60, TimeUnit.SECONDS)
84+
.pollInterval(2, TimeUnit.SECONDS)
85+
.ignoreExceptionsMatching(e -> e instanceof InconsistentDataException)
86+
.untilAsserted(
87+
() -> {
88+
try (ResultSet resultSet =
89+
s.executeQuery("SELECT last s1 FROM root.** ORDER BY TIMESERIES ASC")) {
90+
ResultSetMetaData metaData = resultSet.getMetaData();
91+
assertEquals(4, metaData.getColumnCount());
92+
while (resultSet.next()) {
93+
StringBuilder result = new StringBuilder();
94+
for (int j = 0; j < metaData.getColumnCount(); j++) {
95+
result
96+
.append(metaData.getColumnName(j + 1))
97+
.append(":")
98+
.append(resultSet.getString(j + 1))
99+
.append(",");
100+
}
101+
System.out.println(result);
102+
}
103+
}
104+
});
83105
}
84106
s.execute("INSERT INTO root.db1.d1 (time, s1) VALUES (1, 1)");
85107
s.execute("INSERT INTO root.db2.d1 (time, s1) VALUES (1, 1)");

0 commit comments

Comments
 (0)