OneSizeFitsQuorum commented on code in PR #13178:
URL: https://github.com/apache/iotdb/pull/13178#discussion_r1718083390
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/RegionMaintainHandler.java:
##########
@@ -693,11 +703,22 @@ public void transferRegionLeader(TConsensusGroupId
regionId, TDataNodeLocation o
(CONF.getSchemaRegionRatisRpcLeaderElectionTimeoutMaxMs()
+ CONF.getSchemaRegionRatisRpcLeaderElectionTimeoutMinMs())
/ 2;
+ Integer leaderId =
configManager.getLoadManager().getRegionLeaderMap().get(regionId);
+
+ if (leaderId != -1) {
+ // The migrated node is not leader, so we specify the transfer leader
to current leader node
Review Comment:
then we do not need to send rpc to datanode?
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/RegionMaintainHandler.java:
##########
@@ -667,12 +667,22 @@ public void removeDataNodePersistence(TDataNodeLocation
dataNodeLocation) {
*/
public void transferRegionLeader(TConsensusGroupId regionId,
TDataNodeLocation originalDataNode)
throws ProcedureException, InterruptedException {
+ List<TDataNodeLocation> excludeDataNode = new ArrayList<>();
Review Comment:
Collections.singletonList()
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/RegionMaintainHandler.java:
##########
@@ -453,7 +453,7 @@ public void removeRegionLocation(
getIdWithRpcEndpoint(deprecatedLocation),
status);
configManager.getLoadManager().removeRegionCache(regionId,
deprecatedLocation.getDataNodeId());
-
configManager.getLoadManager().getRouteBalancer().balanceRegionLeaderAndPriority();
+ //
configManager.getLoadManager().getRouteBalancer().balanceRegionLeaderAndPriority();
Review Comment:
do not comment this
##########
iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/ratis/RatisConsensus.java:
##########
@@ -578,15 +597,26 @@ public void resetPeerList(ConsensusGroupId groupId,
List<Peer> peers) throws Con
@Override
public void transferLeader(ConsensusGroupId groupId, Peer newLeader) throws
ConsensusException {
// first fetch the newest information
+ logger.info("transfer consensusGroup {} leader to {}", groupId, newLeader);
final RaftGroupId raftGroupId =
Utils.fromConsensusGroupIdToRaftGroupId(groupId);
final RaftGroup raftGroup =
Optional.ofNullable(getGroupInfo(raftGroupId))
.orElseThrow(() -> new ConsensusGroupNotExistException(groupId));
-
+ if (raftGroup.getPeers() == null) {
Review Comment:
if we forbid leader trasnfer during regionMigration, thus we do not need
this judgement?
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/RegionMaintainHandler.java:
##########
@@ -667,12 +667,22 @@ public void removeDataNodePersistence(TDataNodeLocation
dataNodeLocation) {
*/
public void transferRegionLeader(TConsensusGroupId regionId,
TDataNodeLocation originalDataNode)
throws ProcedureException, InterruptedException {
+ List<TDataNodeLocation> excludeDataNode = new ArrayList<>();
+ excludeDataNode.add(originalDataNode);
Review Comment:
why not use originDataNode directly?
##########
iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/ratis/RatisConsensus.java:
##########
@@ -578,15 +597,26 @@ public void resetPeerList(ConsensusGroupId groupId,
List<Peer> peers) throws Con
@Override
public void transferLeader(ConsensusGroupId groupId, Peer newLeader) throws
ConsensusException {
// first fetch the newest information
+ logger.info("transfer consensusGroup {} leader to {}", groupId, newLeader);
final RaftGroupId raftGroupId =
Utils.fromConsensusGroupIdToRaftGroupId(groupId);
final RaftGroup raftGroup =
Optional.ofNullable(getGroupInfo(raftGroupId))
.orElseThrow(() -> new ConsensusGroupNotExistException(groupId));
-
+ if (raftGroup.getPeers() == null) {
+ throw new ConsensusException("group " + groupId + " has no peers");
+ }
final RaftPeer newRaftLeader =
Utils.fromPeerAndPriorityToRaftPeer(newLeader, DEFAULT_PRIORITY);
-
final RaftClientReply reply;
try {
+ Peer leader = getLeader(groupId);
Review Comment:
same as above, not necessary if we consider this in condignode
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/RegionMaintainHandler.java:
##########
@@ -718,7 +739,7 @@ public void transferRegionLeader(TConsensusGroupId
regionId, TDataNodeLocation o
Collections.singletonMap(
regionId,
new ConsensusGroupHeartbeatSample(timestamp,
newLeaderNode.get().getDataNodeId())));
-
configManager.getLoadManager().getRouteBalancer().balanceRegionLeaderAndPriority();
+ //
configManager.getLoadManager().getRouteBalancer().balanceRegionLeaderAndPriority();
Review Comment:
do not comment
##########
iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/ratis/RatisConsensus.java:
##########
@@ -112,6 +115,11 @@ class RatisConsensus implements IConsensus {
private static final Logger logger =
LoggerFactory.getLogger(RatisConsensus.class);
+ // The two fields are used to control the retry times and wait time for
setConfiguration
+ // reconfiguration may take many time, so we need to set a long wait time
and retry times
+ private final long setConfigurationWaitTime = 10000;
+ private final int setConfigurationRetryTimes = 50;
Review Comment:
not used?
##########
iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/ratis/RatisConsensus.java:
##########
@@ -880,6 +936,30 @@ private RaftClientReply sendReconfiguration(RaftGroup
newGroupConf)
return reply;
}
+ private RaftClientReply sendReconfigurationWithRetry(RaftGroup newGroupConf)
Review Comment:
not used?
##########
iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/ratis/RatisConsensus.java:
##########
@@ -186,6 +195,16 @@ public RatisConsensus(ConsensusConfig config,
IStateMachine.Registry registry) {
this.config.getImpl().getRetryWaitMillis(),
TimeUnit.MILLISECONDS))
.build();
+ regionMigrateRetryPolicy =
Review Comment:
not used?
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/RegionMaintainHandler.java:
##########
@@ -742,12 +763,29 @@ public void transferRegionLeader(TConsensusGroupId
regionId, TDataNodeLocation o
*/
public Optional<TDataNodeLocation> filterDataNodeWithOtherRegionReplica(
TConsensusGroupId regionId, TDataNodeLocation filterLocation) {
+ List<TDataNodeLocation> filterLocations = new ArrayList<>();
+ filterLocations.add(filterLocation);
+ return filterDataNodeWithOtherRegionReplica(regionId, filterLocations);
+ }
+
+ public Optional<TDataNodeLocation> filterDataNodeWithOtherRegionReplica(
+ TConsensusGroupId regionId, List<TDataNodeLocation> filterLocations) {
return filterDataNodeWithOtherRegionReplica(
- regionId, filterLocation, NodeStatus.Running, NodeStatus.ReadOnly);
+ regionId, filterLocations, NodeStatus.Running, NodeStatus.ReadOnly);
}
public Optional<TDataNodeLocation> filterDataNodeWithOtherRegionReplica(
TConsensusGroupId regionId, TDataNodeLocation filterLocation,
NodeStatus... allowingStatus) {
+ List<TDataNodeLocation> excludeLocations = new ArrayList<>();
+ excludeLocations.add(filterLocation);
Review Comment:
same as above
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
