Skip to content

Commit 08a7c89

Browse files
authored
[To dev/1.3] [Test] Fix Ratis mini-cluster shutdown race (#18879) (#18885)
(cherry picked from commit f8135bb)
1 parent 570aafa commit 08a7c89

2 files changed

Lines changed: 119 additions & 2 deletions

File tree

‎iotdb-core/consensus/src/test/java/org/apache/iotdb/consensus/ratis/RecoverReadTest.java‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@
2424
import org.apache.iotdb.consensus.common.request.IConsensusRequest;
2525
import org.apache.iotdb.consensus.config.RatisConfig;
2626
import org.apache.iotdb.consensus.exception.RatisReadUnavailableException;
27+
import org.apache.iotdb.consensus.ratis.utils.Utils;
2728

29+
import org.apache.ratis.server.RaftServer;
2830
import org.apache.ratis.util.TimeDuration;
2931
import org.junit.After;
3032
import org.junit.Assert;
@@ -35,6 +37,10 @@
3537

3638
import java.io.File;
3739
import java.util.List;
40+
import java.util.concurrent.CountDownLatch;
41+
import java.util.concurrent.ExecutorService;
42+
import java.util.concurrent.Executors;
43+
import java.util.concurrent.Future;
3844
import java.util.concurrent.TimeUnit;
3945
import java.util.concurrent.atomic.AtomicBoolean;
4046

@@ -46,6 +52,7 @@ public class RecoverReadTest {
4652
private static class SlowRecoverStateMachine extends TestUtils.IntegerCounter {
4753
private final TimeDuration stallDuration;
4854
private final AtomicBoolean stallApply = new AtomicBoolean(false);
55+
private final CountDownLatch stopped = new CountDownLatch(1);
4956

5057
private SlowRecoverStateMachine(TimeDuration stallDuration) {
5158
this.stallDuration = stallDuration;
@@ -70,6 +77,11 @@ public TSStatus write(IConsensusRequest request) {
7077
return super.write(request);
7178
}
7279

80+
@Override
81+
public void stop() {
82+
stopped.countDown();
83+
}
84+
7385
@Override
7486
public boolean takeSnapshot(File snapshotDir) {
7587
// allow no snapshot to ensure the raft log be replayed
@@ -126,6 +138,64 @@ public void tearUp() throws Exception {
126138
}
127139
}
128140

141+
@Test
142+
public void stopWaitsForInFlightLeaderCheck() throws Exception {
143+
final ConsensusGroupId gid = miniCluster.getGid();
144+
for (RatisConsensus server : miniCluster.getServers()) {
145+
server.createLocalPeer(gid, miniCluster.getPeers());
146+
}
147+
miniCluster.waitUntilActiveLeaderElectedAndReady();
148+
149+
int leaderIndex = 0;
150+
while (!miniCluster.getServer(leaderIndex).isLeader(gid)) {
151+
leaderIndex++;
152+
}
153+
final RaftServer.Division leader =
154+
miniCluster
155+
.getServer(leaderIndex)
156+
.getServer()
157+
.getDivision(Utils.fromConsensusGroupIdToRaftGroupId(gid));
158+
final SlowRecoverStateMachine stateMachine =
159+
(SlowRecoverStateMachine) miniCluster.getStateMachines().get(leaderIndex);
160+
final CountDownLatch checkStarted = new CountDownLatch(1);
161+
final CountDownLatch finishCheck = new CountDownLatch(1);
162+
final CountDownLatch stopStarted = new CountDownLatch(1);
163+
final ExecutorService executor = Executors.newFixedThreadPool(2);
164+
try {
165+
final Future<?> check =
166+
executor.submit(
167+
() -> {
168+
// LeaderStateImpl performs its leadership checks under this monitor.
169+
synchronized (leader) {
170+
checkStarted.countDown();
171+
Assert.assertTrue(finishCheck.await(30, TimeUnit.SECONDS));
172+
leader.getRaftLog().getLastEntryTermIndex();
173+
}
174+
return null;
175+
});
176+
Assert.assertTrue(checkStarted.await(30, TimeUnit.SECONDS));
177+
final Future<?> stop =
178+
executor.submit(
179+
() -> {
180+
stopStarted.countDown();
181+
miniCluster.stop();
182+
return null;
183+
});
184+
Assert.assertTrue(stopStarted.await(30, TimeUnit.SECONDS));
185+
Assert.assertFalse(
186+
"The state machine must remain open until the in-flight leader check finishes",
187+
stateMachine.stopped.await(5, TimeUnit.SECONDS));
188+
finishCheck.countDown();
189+
check.get(30, TimeUnit.SECONDS);
190+
stop.get(30, TimeUnit.SECONDS);
191+
Assert.assertEquals(0, stateMachine.stopped.getCount());
192+
} finally {
193+
finishCheck.countDown();
194+
executor.shutdown();
195+
Assert.assertTrue(executor.awaitTermination(30, TimeUnit.SECONDS));
196+
}
197+
}
198+
129199
/* mimics the situation before this patch */
130200
@Test
131201
public void inconsistentReadAfterRestart() throws Exception {

‎iotdb-core/consensus/src/test/java/org/apache/iotdb/consensus/ratis/TestUtils.java‎

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,16 @@
3939
import org.apache.iotdb.consensus.ratis.utils.Retriable;
4040
import org.apache.iotdb.consensus.ratis.utils.Utils;
4141

42+
import org.apache.ratis.protocol.ClientId;
43+
import org.apache.ratis.protocol.LeaderElectionManagementRequest;
44+
import org.apache.ratis.protocol.RaftClientReply;
45+
import org.apache.ratis.protocol.RaftGroupId;
46+
import org.apache.ratis.protocol.TransferLeadershipRequest;
47+
import org.apache.ratis.server.RaftServer;
4248
import org.apache.ratis.thirdparty.com.google.common.base.Preconditions;
4349
import org.apache.ratis.util.ExitUtils;
4450
import org.apache.ratis.util.FileUtils;
51+
import org.apache.ratis.util.LifeCycle;
4552
import org.apache.ratis.util.TimeDuration;
4653
import org.apache.ratis.util.Timestamp;
4754
import org.junit.Assert;
@@ -305,13 +312,53 @@ void start() throws IOException {
305312
}
306313

307314
void stop() throws IOException {
308-
for (RatisConsensus server : servers) {
309-
server.stop();
315+
if (isStopped.get()) {
316+
return;
317+
}
318+
try {
319+
stepDownBeforeStopping();
320+
} finally {
321+
for (RatisConsensus server : servers) {
322+
server.stop();
323+
}
310324
}
311325
waitUntilPortsReleased(peerPorts);
312326
isStopped.set(true);
313327
}
314328

329+
private void stepDownBeforeStopping() throws IOException {
330+
final ClientId clientId = ClientId.randomId();
331+
final List<RaftServer> runningServers = new ArrayList<>();
332+
// Ratis 3.2.1 closes the log without joining the leader event processor. Step down through
333+
// that processor first, so an in-flight leadership check cannot access an already closed log.
334+
// Pause elections on every peer before stepping down to prevent a replacement leader.
335+
for (RatisConsensus server : servers) {
336+
final RaftServer raftServer = server.getServer();
337+
if (raftServer.getLifeCycleState() != LifeCycle.State.RUNNING) {
338+
continue;
339+
}
340+
runningServers.add(raftServer);
341+
for (RaftGroupId groupId : raftServer.getGroupIds()) {
342+
final RaftClientReply reply =
343+
raftServer.leaderElectionManagement(
344+
LeaderElectionManagementRequest.newPause(
345+
clientId, raftServer.getId(), groupId, 0));
346+
Assert.assertTrue(reply.toString(), reply.isSuccess());
347+
}
348+
}
349+
for (RaftServer raftServer : runningServers) {
350+
for (RaftGroupId groupId : raftServer.getGroupIds()) {
351+
if (raftServer.getDivision(groupId).getInfo().isLeader()) {
352+
final RaftClientReply reply =
353+
raftServer.transferLeadership(
354+
new TransferLeadershipRequest(
355+
clientId, raftServer.getId(), groupId, 1, null, 10_000));
356+
Assert.assertTrue(reply.toString(), reply.isSuccess());
357+
}
358+
}
359+
}
360+
}
361+
315362
void cleanUp() throws IOException {
316363
stop();
317364
for (File storage : peerStorage) {

0 commit comments

Comments
 (0)