Visitar URL original
Reject invalid multi-region submissions in ITs · apache/iotdb@99f9c34 · GitHub
Skip to content

Commit 99f9c34

Browse files
CRZbulabulaCRZbulabula
committed
Reject invalid multi-region submissions in ITs
1 parent 3e6b8eb commit 99f9c34

2 files changed

Lines changed: 56 additions & 76 deletions

File tree

‎integration-test/src/test/java/org/apache/iotdb/confignode/it/regionmigration/pass/commit/IoTDBMigrateMultiRegionForIoTV1IT.java‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.slf4j.LoggerFactory;
3737

3838
import java.sql.Connection;
39+
import java.sql.SQLException;
3940
import java.sql.Statement;
4041
import java.util.ArrayList;
4142
import java.util.List;
@@ -125,14 +126,8 @@ public void multiRegionMigrateTest() throws Exception {
125126
try {
126127
statement.execute(command);
127128
return true;
128-
} catch (Exception e) {
129+
} catch (SQLException e) {
129130
String errorMessage = e.getMessage();
130-
if (errorMessage != null
131-
&& errorMessage.contains("successfully submitted")
132-
&& errorMessage.contains("failed to submit")) {
133-
LOGGER.warn("Multi-region migrate partially succeeded: {}", errorMessage);
134-
return true;
135-
}
136131
LOGGER.warn("Multi-region migrate failed, retrying: {}", errorMessage);
137132
return false;
138133
}

‎integration-test/src/test/java/org/apache/iotdb/confignode/it/regionmigration/pass/commit/IoTDBRegionGroupExpandAndShrinkForIoTV1IT.java‎

Lines changed: 54 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.apache.iotdb.it.env.EnvFactory;
2727
import org.apache.iotdb.it.framework.IoTDBTestRunner;
2828
import org.apache.iotdb.itbase.category.ClusterIT;
29+
import org.apache.iotdb.rpc.TSStatusCode;
2930

3031
import org.awaitility.Awaitility;
3132
import org.junit.Assert;
@@ -168,14 +169,13 @@ public void extendRegionToInvalidDataNodeTest() throws Exception {
168169
Assert.assertFalse(
169170
"ConfigNode should not throw NullPointerException, but got: " + message,
170171
message.contains("NullPointerException"));
171-
// ... and the submission must be rejected cleanly. "extend region" wraps every region's
172-
// result, so the top-level message only reports the aggregate counts; the concrete "does
173-
// not
174-
// exist in the cluster" reason is carried in the per-region sub-status.
172+
// The operation-specific error and the concrete reason must reach the JDBC client.
173+
Assert.assertEquals(TSStatusCode.EXTEND_REGION_ERROR.getStatusCode(), e.getErrorCode());
175174
Assert.assertTrue(
176175
"Expected the extend submission to be rejected but got: " + message,
177-
message.contains("failed to submit: 1"));
176+
message.contains("Target DataNode " + invalidDataNodeId + " does not exist"));
178177
}
178+
assertRegionMapUnchanged(statement, regionMap);
179179
}
180180
}
181181

@@ -314,7 +314,7 @@ public void multiRegionNormalTest() throws Exception {
314314
}
315315
}
316316

317-
/** Test multi-region expand with partial regions already in target DataNode */
317+
/** Reject the entire expansion when a later region already exists on the target DataNode. */
318318
@Test
319319
public void multiRegionExpandPartialExistTest() throws Exception {
320320
EnvFactory.getEnv()
@@ -338,49 +338,42 @@ public void multiRegionExpandPartialExistTest() throws Exception {
338338
Map<Integer, Set<Integer>> regionMap = getAllRegionMap(statement);
339339
Set<Integer> allDataNodeId = getAllDataNodes(statement);
340340

341-
List<Integer> allRegions = new ArrayList<>(regionMap.keySet());
342-
List<Integer> selectedRegions = allRegions.subList(0, Math.min(3, allRegions.size()));
341+
Assert.assertEquals(2, regionMap.size());
342+
List<Integer> selectedRegions = new ArrayList<>(regionMap.keySet());
343343

344344
int targetDataNode =
345345
findDataNodeNotContainsAnyRegion(allDataNodeId, regionMap, selectedRegions);
346346

347-
// first expand some regions individually
348-
List<Integer> preExpandRegions =
349-
selectedRegions.subList(0, Math.min(2, selectedRegions.size()));
350-
for (int regionId : preExpandRegions) {
351-
regionGroupExpand(statement, client, regionId, targetDataNode);
352-
}
353-
354-
// now try to expand all regions (including already expanded ones)
355-
LOGGER.info(
356-
"Testing multi-expand with regions {} to DataNode {}, where {} already exist",
357-
selectedRegions,
358-
targetDataNode,
359-
preExpandRegions);
360-
361-
multiRegionGroupExpand(statement, client, selectedRegions, targetDataNode);
362-
363-
// verify all regions are in target DataNode
347+
// Keep the first region valid so this also catches submission during validation.
348+
regionGroupExpand(statement, client, selectedRegions.get(1), targetDataNode);
364349
regionMap = getAllRegionMap(statement);
365-
for (int regionId : selectedRegions) {
366-
Assert.assertTrue(
367-
"Region " + regionId + " should contain target DataNode " + targetDataNode,
368-
regionMap.get(regionId).contains(targetDataNode));
369-
}
370-
LOGGER.info("Multi-region expand partial exist test passed");
350+
Assert.assertFalse(regionMap.get(selectedRegions.get(0)).contains(targetDataNode));
351+
Assert.assertTrue(regionMap.get(selectedRegions.get(1)).contains(targetDataNode));
352+
353+
SQLException exception =
354+
Assert.assertThrows(
355+
SQLException.class,
356+
() ->
357+
statement.execute(
358+
buildMultiRegionCommand(
359+
MULTI_EXPAND_FORMAT, selectedRegions, targetDataNode)));
360+
Assert.assertEquals(
361+
TSStatusCode.EXTEND_REGION_ERROR.getStatusCode(), exception.getErrorCode());
362+
Assert.assertTrue(exception.getMessage().contains("already contains region"));
363+
assertRegionMapUnchanged(statement, regionMap);
371364
}
372365
}
373366

374-
/** Test multi-region shrink with partial regions not in target DataNode */
367+
/** Reject the entire removal when a later region does not exist on the target DataNode. */
375368
@Test
376369
public void multiRegionShrinkPartialNotExistTest() throws Exception {
377370
EnvFactory.getEnv()
378371
.getConfig()
379372
.getCommonConfig()
380373
.setDataRegionConsensusProtocolClass(ConsensusFactory.IOT_CONSENSUS)
381374
.setSchemaRegionConsensusProtocolClass(ConsensusFactory.RATIS_CONSENSUS)
382-
.setDataReplicationFactor(1)
383-
.setSchemaReplicationFactor(1);
375+
.setDataReplicationFactor(2)
376+
.setSchemaReplicationFactor(2);
384377

385378
EnvFactory.getEnv().initClusterEnvironment(1, 5);
386379

@@ -395,42 +388,43 @@ public void multiRegionShrinkPartialNotExistTest() throws Exception {
395388
Map<Integer, Set<Integer>> regionMap = getAllRegionMap(statement);
396389
Set<Integer> allDataNodeId = getAllDataNodes(statement);
397390

398-
List<Integer> allRegions = new ArrayList<>(regionMap.keySet());
399-
List<Integer> selectedRegions = allRegions.subList(0, Math.min(3, allRegions.size()));
391+
Assert.assertEquals(2, regionMap.size());
392+
List<Integer> selectedRegions = new ArrayList<>(regionMap.keySet());
400393

401394
int targetDataNode =
402395
findDataNodeNotContainsAnyRegion(allDataNodeId, regionMap, selectedRegions);
403396

404397
// first expand all regions to target DataNode
405398
multiRegionGroupExpand(statement, client, selectedRegions, targetDataNode);
406399

407-
// then shrink some regions individually
408-
List<Integer> preShrinkRegions =
409-
selectedRegions.subList(0, Math.min(2, selectedRegions.size()));
410-
for (int regionId : preShrinkRegions) {
411-
regionGroupShrink(statement, client, regionId, targetDataNode);
412-
}
413-
414-
// now try to shrink all regions (including already shrunk ones)
415-
LOGGER.info(
416-
"Testing multi-shrink with regions {} from DataNode {}, where {} already removed",
417-
selectedRegions,
418-
targetDataNode,
419-
preShrinkRegions);
420-
421-
multiRegionGroupShrink(statement, client, selectedRegions, targetDataNode);
422-
423-
// verify all regions are not in target DataNode
400+
// Keep the first region valid and leave two replicas of the second region elsewhere.
401+
regionGroupShrink(statement, client, selectedRegions.get(1), targetDataNode);
424402
regionMap = getAllRegionMap(statement);
425-
for (int regionId : selectedRegions) {
426-
Assert.assertFalse(
427-
"Region " + regionId + " should not contain target DataNode " + targetDataNode,
428-
regionMap.get(regionId).contains(targetDataNode));
429-
}
430-
LOGGER.info("Multi-region shrink partial not exist test passed");
403+
Assert.assertTrue(regionMap.get(selectedRegions.get(0)).contains(targetDataNode));
404+
Assert.assertFalse(regionMap.get(selectedRegions.get(1)).contains(targetDataNode));
405+
406+
SQLException exception =
407+
Assert.assertThrows(
408+
SQLException.class,
409+
() ->
410+
statement.execute(
411+
buildMultiRegionCommand(
412+
MULTI_SHRINK_FORMAT, selectedRegions, targetDataNode)));
413+
Assert.assertEquals(
414+
TSStatusCode.REMOVE_REGION_PEER_ERROR.getStatusCode(), exception.getErrorCode());
415+
Assert.assertTrue(exception.getMessage().contains("doesn't contain Region"));
416+
assertRegionMapUnchanged(statement, regionMap);
431417
}
432418
}
433419

420+
private void assertRegionMapUnchanged(
421+
Statement statement, Map<Integer, Set<Integer>> expectedRegionMap) {
422+
Awaitility.await()
423+
.during(3, TimeUnit.SECONDS)
424+
.atMost(10, TimeUnit.SECONDS)
425+
.untilAsserted(() -> Assert.assertEquals(expectedRegionMap, getAllRegionMap(statement)));
426+
}
427+
434428
private void multiRegionGroupExpand(
435429
Statement statement,
436430
SyncConfigNodeIServiceClient client,
@@ -519,17 +513,8 @@ private void executeMultiRegionOperation(
519513
try {
520514
statement.execute(command);
521515
return true;
522-
} catch (Exception e) {
516+
} catch (SQLException e) {
523517
String errorMessage = e.getMessage();
524-
// If error message contains both "successfully submitted" and "failed to submit",
525-
// consider it as partial success and continue
526-
if (errorMessage != null
527-
&& errorMessage.contains("successfully submitted")
528-
&& errorMessage.contains("failed to submit")) {
529-
LOGGER.warn(
530-
"Multi-region {} partially succeeded: {}", operationType, errorMessage);
531-
return true;
532-
}
533518
LOGGER.warn(
534519
"Multi-region {} command execution failed, retrying: {}",
535520
operationType,

0 commit comments

Comments
 (0)