Skip to content

Commit 20cab12

Browse files
authored
[Fix] Prevent stale schema queue closes after leader transfer (#18948)
* [Fix] Prevent stale schema queue closes from following leader transfers * [Pipe] Wait for historical transfer before asserting mods
1 parent 4fceb75 commit 20cab12

6 files changed

Lines changed: 143 additions & 0 deletions

File tree

‎integration-test/src/test/java/org/apache/iotdb/pipe/it/dual/treemodel/auto/basic/IoTDBPipeDataSinkIT.java‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,18 @@
4141

4242
import java.nio.charset.Charset;
4343
import java.sql.Connection;
44+
import java.sql.ResultSet;
4445
import java.sql.Statement;
4546
import java.util.Arrays;
4647
import java.util.Collections;
4748
import java.util.HashMap;
4849
import java.util.HashSet;
4950
import java.util.Map;
51+
import java.util.concurrent.TimeUnit;
5052
import java.util.function.Consumer;
5153

5254
import static org.apache.iotdb.db.it.IoTDBRestServiceIT.getHttpPost;
55+
import static org.awaitility.Awaitility.await;
5356

5457
@RunWith(IoTDBTestRunner.class)
5558
@Category({MultiClusterIT2DualTreeAutoBasic.class})
@@ -582,6 +585,7 @@ public void testTransferMods() {
582585
"create pipe test with source ('source.realtime.mode'='stream','inclusion'='data','path'='root.sg_nonAligned.非对齐序列带有encoding和压缩方式.**','source.realtime.enable'='true','mods.enable'='true') with sink ('sink'='iotdb-thrift-sink', 'sink.node-urls'='%s')",
583586
receiverEnv.getDataNodeWrapperList().get(0).getIpAndPortString())));
584587

588+
awaitHistoricalPipeCompletion("test");
585589
TestUtils.assertDataEventuallyOnEnv(
586590
receiverEnv,
587591
"count timeseries root.sg_nonAligned.非对齐序列带有encoding和压缩方式.*",
@@ -602,6 +606,7 @@ public void testTransferMods() {
602606
"create pipe test with source ('source.realtime.mode'='stream','inclusion'='data','path'='root.sg_nonAligned.非对齐序列带有encoding和压缩方式.**','source.realtime.enable'='true','mods.enable'='true') with sink ('sink'='iotdb-thrift-sink', 'sink.node-urls'='%s')",
603607
receiverEnv.getDataNodeWrapperList().get(0).getIpAndPortString())));
604608

609+
awaitHistoricalPipeCompletion("test");
605610
TestUtils.assertDataEventuallyOnEnv(
606611
receiverEnv,
607612
"count timeseries root.sg_nonAligned.非对齐序列带有encoding和压缩方式.*",
@@ -614,4 +619,38 @@ public void testTransferMods() {
614619
"count(timeseries),",
615620
Collections.singleton("4,"));
616621
}
622+
623+
private void awaitHistoricalPipeCompletion(final String pipeName) {
624+
await()
625+
.pollInSameThread()
626+
.pollDelay(1L, TimeUnit.SECONDS)
627+
.pollInterval(1L, TimeUnit.SECONDS)
628+
.atMost(600L, TimeUnit.SECONDS)
629+
.untilAsserted(
630+
() -> {
631+
boolean sourcePipeFound = false;
632+
boolean sourcePipeCompleted = true;
633+
boolean realtimePipeFound = false;
634+
635+
for (final DataNodeWrapper dataNode : senderEnv.getDataNodeWrapperList()) {
636+
try (final Connection connection = senderEnv.getConnection(dataNode);
637+
final Statement statement = connection.createStatement();
638+
final ResultSet resultSet = statement.executeQuery("show pipes")) {
639+
while (resultSet.next()) {
640+
final String id = resultSet.getString(1);
641+
if (id.equals(pipeName) || id.equals(pipeName + "_history")) {
642+
sourcePipeFound = true;
643+
sourcePipeCompleted &= "0".equals(resultSet.getString(8));
644+
} else if (id.equals(pipeName + "_realtime")) {
645+
realtimePipeFound = true;
646+
}
647+
}
648+
}
649+
}
650+
651+
// The historical sub-pipe may be auto-dropped after all events are committed.
652+
Assert.assertTrue(sourcePipeFound || realtimePipeFound);
653+
Assert.assertTrue(!sourcePipeFound || sourcePipeCompleted);
654+
});
655+
}
617656
}

‎iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/ratis/RatisConsensus.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,12 @@ && waitUntilLeaderReady(raftGroupId)) {
373373
}
374374
}
375375

376+
// A leader-local decision may already be stale after a leadership change. Let the current
377+
// leader make its own decision instead of replaying the old leader's request there.
378+
if (!request.isForwardable()) {
379+
return StatusUtils.getStatus(TSStatusCode.REDIRECTION_RECOMMEND);
380+
}
381+
376382
// 2. try raft client
377383
TSStatus writeResult;
378384
try (AutoCloseable ignored =

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
package org.apache.iotdb.consensus.ratis;
2121

2222
import org.apache.iotdb.commons.consensus.ConsensusGroupId;
23+
import org.apache.iotdb.commons.request.IConsensusRequest;
2324
import org.apache.iotdb.consensus.IStateMachine;
2425
import org.apache.iotdb.consensus.common.ConsensusGroup;
2526
import org.apache.iotdb.consensus.common.Peer;
@@ -29,6 +30,7 @@
2930
import org.apache.iotdb.consensus.exception.ConsensusGroupNotExistException;
3031
import org.apache.iotdb.consensus.exception.PeerAlreadyInConsensusGroupException;
3132
import org.apache.iotdb.consensus.exception.PeerNotInConsensusGroupException;
33+
import org.apache.iotdb.rpc.TSStatusCode;
3234

3335
import org.apache.ratis.util.TimeDuration;
3436
import org.junit.After;
@@ -113,6 +115,44 @@ public void basicConsensus3Copy() throws Exception {
113115
doConsensus(0, 10, 10);
114116
}
115117

118+
@Test
119+
public void testNonForwardableWriteAfterLeaderTransfer() throws Exception {
120+
for (RatisConsensus server : servers) {
121+
server.createLocalPeer(gid, peers);
122+
}
123+
miniCluster.waitUntilActiveLeaderElectedAndReady();
124+
125+
final int oldLeaderId = servers.get(0).getLeader(gid).getNodeId();
126+
final int newLeaderId = (oldLeaderId + 1) % servers.size();
127+
final RatisConsensus oldLeader = servers.get(oldLeaderId);
128+
final IConsensusRequest leaderLocalRequest =
129+
new TestUtils.TestRequest(TestUtils.TestRequest.incrRequest().serializeToByteBuffer()) {
130+
@Override
131+
public boolean isForwardable() {
132+
return false;
133+
}
134+
};
135+
136+
oldLeader.transferLeader(gid, peers.get(newLeaderId));
137+
miniCluster.waitUntilActiveLeaderElectedAndReady();
138+
Assert.assertEquals(newLeaderId, oldLeader.getLeader(gid).getNodeId());
139+
140+
// The new leader has already made its own decision before the old request arrives.
141+
Assert.assertEquals(
142+
TSStatusCode.SUCCESS_STATUS.getStatusCode(),
143+
servers.get(newLeaderId).write(gid, leaderLocalRequest).getCode());
144+
Assert.assertEquals(
145+
TSStatusCode.REDIRECTION_RECOMMEND.getStatusCode(),
146+
oldLeader.write(gid, leaderLocalRequest).getCode());
147+
Assert.assertEquals(1, miniCluster.mustRead(newLeaderId));
148+
149+
// Ordinary writes issued to a follower must still reach the current leader.
150+
Assert.assertEquals(
151+
TSStatusCode.SUCCESS_STATUS.getStatusCode(),
152+
oldLeader.write(gid, TestUtils.TestRequest.incrRequest()).getCode());
153+
Assert.assertEquals(2, miniCluster.mustRead(newLeaderId));
154+
}
155+
116156
@Test
117157
public void addMemberToGroup() throws Exception {
118158
List<Peer> original = peers.subList(0, 1);

‎iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/planner/plan/node/pipe/PipeOperateSchemaQueueNode.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,13 @@ public boolean isOpen() {
5454
return isOpen;
5555
}
5656

57+
@Override
58+
public boolean isForwardable() {
59+
// A close is decided from the local pipe metadata. After a leader change the new leader may
60+
// already have newer metadata and an active source, so an old close must not be forwarded.
61+
return isOpen;
62+
}
63+
5764
@Override
5865
public List<PlanNode> getChildren() {
5966
return Collections.emptyList();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.apache.iotdb.db.queryengine.plan.planner.plan.node.pipe;
21+
22+
import org.apache.iotdb.commons.queryengine.plan.planner.plan.node.PlanNodeId;
23+
import org.apache.iotdb.commons.queryengine.plan.planner.plan.node.PlanNodeType;
24+
25+
import org.junit.Assert;
26+
import org.junit.Test;
27+
28+
public class PipeOperateSchemaQueueNodeTest {
29+
30+
@Test
31+
public void testCloseCannotBeForwardedToNewLeader() {
32+
final PipeOperateSchemaQueueNode close =
33+
new PipeOperateSchemaQueueNode(new PlanNodeId("close"), false);
34+
Assert.assertFalse(close.isForwardable());
35+
Assert.assertFalse(close.clone().isForwardable());
36+
Assert.assertFalse(PlanNodeType.deserialize(close.serializeToByteBuffer()).isForwardable());
37+
38+
final PipeOperateSchemaQueueNode open =
39+
new PipeOperateSchemaQueueNode(new PlanNodeId("open"), true);
40+
Assert.assertTrue(open.isForwardable());
41+
Assert.assertTrue(PlanNodeType.deserialize(open.serializeToByteBuffer()).isForwardable());
42+
}
43+
}

‎iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/request/IConsensusRequest.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,14 @@ default long getMemorySize() {
4141
return 0;
4242
}
4343

44+
/**
45+
* Whether a consensus implementation may forward this write to another leader. Requests based on
46+
* the local leader's runtime state must not be forwarded after that leader steps down.
47+
*/
48+
default boolean isForwardable() {
49+
return true;
50+
}
51+
4452
/**
4553
* Returns whether this request materializes its bytes only when it is about to be sent, instead
4654
* of when it is handed over to the replication layer.

0 commit comments

Comments
 (0)