Skip to content

Commit 08d7dfe

Browse files
committed
Fix flaky datanode UTs under parallel surefire forks
Two intermittent failures observed in ut-datanode (forkCount>1): 1. DataRegionTest.tearDown:165 "failed to close some ports" EnvironmentUtils.examinePorts() checks 6667/5555/31999/9091 machine-wide via connect(). DataNodeInternalRPCServiceImplTest legitimately binds 6667 through DataRegionConsensusImpl.start() -> IoTConsensusRPCService. A sibling fork's tearDown then sees 6667 open and fails -- a cross-fork false positive, not a real leak. This was missed by #17698's premise ("zero ServerSocket / bind() / TServer.serve() calls in tests") because the binding happens indirectly through IConsensus.start(). 2. AlignedSeriesScanOperatorTest.batchTest2:423 NPE on null TsBlock FullOuterTimeJoinOperator.next() (and AbstractSeriesScanOperator.next()) legitimately return null or an empty TsBlock when prepareInput() yields on its 500ms maxRunTime slice. The test dereferenced tsBlock immediately. Fixes: - Introduce EnvironmentUtils.FORK_PORT_OFFSET = ((surefire.forkNumber - 1) % 30 + 1) * 1000. Each fork shifts its bound ports AND the corresponding examinePorts() check by this offset, so forks live in disjoint port namespaces. examinePorts() in fork A only sees ports bound by fork A (real leak detection preserved); fork A no longer sees fork B's bindings (cross-fork false positive removed). Modulo 30 caps the offset so the highest checked port (31999 + 30*1000 = 61999) stays valid. - DataNodeInternalRPCServiceImplTest binds 6667 + FORK_PORT_OFFSET via the same constant -- binding and check stay in sync. - AlignedSeriesScanOperatorTest.batchTest1/2/3: add `if (tsBlock == null || tsBlock.isEmpty()) continue;` after each operator.next(), matching the pattern in TreeTopKOperatorTest, MergeTreeSortOperatorTest, TreeSortOperatorTest, etc. Audited iotdb-core/datanode UTs for other monitored port bindings -- none beyond DataNodeInternalRPCServiceImplTest.
1 parent 4ac0b7e commit 08d7dfe

3 files changed

Lines changed: 46 additions & 10 deletions

File tree

iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/execution/operator/AlignedSeriesScanOperatorTest.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,9 @@ public void batchTest1() throws Exception {
140140
int count = 0;
141141
while (seriesScanOperator.hasNext()) {
142142
TsBlock tsBlock = seriesScanOperator.next();
143+
if (tsBlock == null || tsBlock.isEmpty()) {
144+
continue;
145+
}
143146
assertEquals(6, tsBlock.getValueColumnCount());
144147
assertTrue(tsBlock.getColumn(0) instanceof BooleanColumn);
145148
assertTrue(tsBlock.getColumn(1) instanceof IntColumn);
@@ -420,6 +423,9 @@ public void batchTest2() throws Exception {
420423
int count = 0;
421424
while (timeJoinOperator.isBlocked().isDone() && timeJoinOperator.hasNext()) {
422425
TsBlock tsBlock = timeJoinOperator.next();
426+
if (tsBlock == null || tsBlock.isEmpty()) {
427+
continue;
428+
}
423429
assertEquals(18, tsBlock.getValueColumnCount());
424430
assertTrue(tsBlock.getColumn(0) instanceof BooleanColumn);
425431
assertTrue(tsBlock.getColumn(1) instanceof IntColumn);
@@ -727,6 +733,9 @@ public void batchTest3() throws Exception {
727733
int count = 499;
728734
while (timeJoinOperator.isBlocked().isDone() && timeJoinOperator.hasNext()) {
729735
TsBlock tsBlock = timeJoinOperator.next();
736+
if (tsBlock == null || tsBlock.isEmpty()) {
737+
continue;
738+
}
730739
assertEquals(18, tsBlock.getValueColumnCount());
731740
assertTrue(tsBlock.getColumn(0) instanceof BooleanColumn);
732741
assertTrue(tsBlock.getColumn(1) instanceof IntColumn);

iotdb-core/datanode/src/test/java/org/apache/iotdb/db/service/DataNodeInternalRPCServiceImplTest.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,12 @@ public class DataNodeInternalRPCServiceImplTest {
9393
private static final File storageDir = new File("target" + java.io.File.separator + "impl");
9494
private static DataRegion dataRegion;
9595

96+
// Each parallel surefire fork binds its own consensus port. Reuses
97+
// EnvironmentUtils.FORK_PORT_OFFSET so this binding and examinePorts() are
98+
// guaranteed to look at the same port — fork-local leaks still fail, sibling
99+
// forks no longer cause cross-fork false positives.
100+
private static final int CONSENSUS_PORT = 6667 + EnvironmentUtils.FORK_PORT_OFFSET;
101+
96102
@BeforeClass
97103
public static void setUpBeforeClass() throws IOException, MetadataException, ConsensusException {
98104
// In standalone mode, we need to set dataNodeId to 0 for RaftPeerId in RatisConsensus
@@ -109,7 +115,7 @@ public static void setUpBeforeClass() throws IOException, MetadataException, Con
109115
ConsensusFactory.IOT_CONSENSUS,
110116
ConsensusConfig.newBuilder()
111117
.setThisNodeId(1)
112-
.setThisNode(new TEndPoint("0.0.0.0", 6667))
118+
.setThisNode(new TEndPoint("0.0.0.0", CONSENSUS_PORT))
113119
.setStorageDir(storageDir.getAbsolutePath())
114120
.setConsensusGroupType(TConsensusGroupType.DataRegion)
115121
.build(),
@@ -124,7 +130,8 @@ public static void setUpBeforeClass() throws IOException, MetadataException, Con
124130
((IoTConsensus) DataRegionConsensusImpl.getInstance()).getImpl(new DataRegionId(1)))) {
125131
DataRegionConsensusImpl.getInstance()
126132
.createLocalPeer(
127-
id, Collections.singletonList(new Peer(id, 1, new TEndPoint("0.0.0.0", 6667))));
133+
id,
134+
Collections.singletonList(new Peer(id, 1, new TEndPoint("0.0.0.0", CONSENSUS_PORT))));
128135
}
129136
DataRegionConsensusImpl.getInstance().start();
130137
SchemaRegionConsensusImpl.getInstance().start();

iotdb-core/datanode/src/test/java/org/apache/iotdb/db/utils/EnvironmentUtils.java

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,17 @@ public class EnvironmentUtils {
9696
public static boolean examinePorts =
9797
Boolean.parseBoolean(System.getProperty("test.port.closed", "false"));
9898

99+
// Per-fork port namespace. Each surefire fork shifts every test-bound port
100+
// (and the corresponding examinePorts() check) by FORK_PORT_OFFSET so that
101+
// parallel forks live in disjoint port spaces. This keeps the leak-detection
102+
// power of examinePorts() (a fork still sees its own un-closed ports) while
103+
// removing the cross-fork false positive (one fork's bound port no longer
104+
// looks like another fork's leak). surefire.forkNumber starts at 1; defaults
105+
// to 1 outside surefire (IDE). Modulo 30 caps the offset so the highest
106+
// checked port (31999 + 30*1000 = 61999) stays within range.
107+
public static final int FORK_PORT_OFFSET =
108+
((Integer.parseInt(System.getProperty("surefire.forkNumber", "1")) - 1) % 30 + 1) * 1000;
109+
99110
public static void cleanEnv() throws IOException, StorageEngineException {
100111
// wait all compaction finished
101112
CompactionTaskManager.getInstance().waitAllCompactionFinish();
@@ -174,23 +185,31 @@ public static void cleanEnv() throws IOException, StorageEngineException {
174185
}
175186

176187
private static boolean examinePorts() {
177-
TTransport transport = TSocketWrapper.wrap(tConfiguration, "127.0.0.1", 6667, 100);
188+
// All checks use this fork's port namespace (base + FORK_PORT_OFFSET) so
189+
// that we only ever see ports bound by THIS JVM — sibling forks live in
190+
// disjoint namespaces.
191+
int rpcPort = 6667 + FORK_PORT_OFFSET;
192+
int syncPort = 5555 + FORK_PORT_OFFSET;
193+
int jmxPort = 31999 + FORK_PORT_OFFSET;
194+
int metricPort = 9091 + FORK_PORT_OFFSET;
195+
196+
TTransport transport = TSocketWrapper.wrap(tConfiguration, "127.0.0.1", rpcPort, 100);
178197
if (transport != null && !transport.isOpen()) {
179198
try {
180199
transport.open();
181-
logger.error("stop daemon failed. 6667 can be connected now.");
200+
logger.error("stop daemon failed. {} can be connected now.", rpcPort);
182201
transport.close();
183202
return false;
184203
} catch (TTransportException e) {
185204
// do nothing
186205
}
187206
}
188207
// try sync service
189-
transport = TSocketWrapper.wrap(tConfiguration, "127.0.0.1", 5555, 100);
208+
transport = TSocketWrapper.wrap(tConfiguration, "127.0.0.1", syncPort, 100);
190209
if (transport != null && !transport.isOpen()) {
191210
try {
192211
transport.open();
193-
logger.error("stop Sync daemon failed. 5555 can be connected now.");
212+
logger.error("stop Sync daemon failed. {} can be connected now.", syncPort);
194213
transport.close();
195214
return false;
196215
} catch (TTransportException e) {
@@ -199,18 +218,19 @@ private static boolean examinePorts() {
199218
}
200219
// try jmx connection
201220
try {
202-
JMXServiceURL url = new JMXServiceURL("service:jmx:rmi:///jndi/rmi://localhost:31999/jmxrmi");
221+
JMXServiceURL url =
222+
new JMXServiceURL("service:jmx:rmi:///jndi/rmi://localhost:" + jmxPort + "/jmxrmi");
203223
JMXConnector jmxConnector = JMXConnectorFactory.connect(url);
204-
logger.error("stop JMX failed. 31999 can be connected now.");
224+
logger.error("stop JMX failed. {} can be connected now.", jmxPort);
205225
jmxConnector.close();
206226
return false;
207227
} catch (IOException e) {
208228
// do nothing
209229
}
210230
// try MetricService
211231
try (Socket socket = new Socket()) {
212-
socket.connect(new InetSocketAddress("127.0.0.1", 9091), 100);
213-
logger.error("stop MetricService failed. 9091 can be connected now.");
232+
socket.connect(new InetSocketAddress("127.0.0.1", metricPort), 100);
233+
logger.error("stop MetricService failed. {} can be connected now.", metricPort);
214234
return false;
215235
} catch (Exception e) {
216236
// do nothing

0 commit comments

Comments
 (0)