OneSizeFitsQuorum commented on code in PR #13178:
URL: https://github.com/apache/iotdb/pull/13178#discussion_r1718189670
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/region/RemoveRegionPeerProcedure.java:
##########
@@ -68,12 +69,25 @@ public RemoveRegionPeerProcedure(
TConsensusGroupId consensusGroupId,
TDataNodeLocation coordinator,
TDataNodeLocation targetDataNode) {
- super();
this.consensusGroupId = consensusGroupId;
this.coordinator = coordinator;
this.targetDataNode = targetDataNode;
}
+ private void handleTransferLeader(RegionMaintainHandler handler)
+ throws ProcedureException, InterruptedException {
+ LOGGER.info(
+ "[pid{}][RemoveRegion] started, region {} will be removed from
DataNode {}.",
+ getProcId(),
+ consensusGroupId.getId(),
+ targetDataNode.getDataNodeId());
+ handler.forceUpdateRegionCache(consensusGroupId, targetDataNode,
RegionStatus.Removing);
+ List<TDataNodeLocation> excludeDataNode = new ArrayList<>();
+ excludeDataNode.add(targetDataNode);
+ excludeDataNode.add(coordinator);
Review Comment:
It feels like the semantics here should be allowed, and in the end, if the
judgment is the same, do not send rpc?
##########
iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/ratis/RatisConsensus.java:
##########
@@ -869,9 +873,34 @@ private RaftClientReply sendReconfiguration(RaftGroup
newGroupConf)
// notify the group leader of configuration change
RaftClientReply reply;
try (RatisClient client = getRaftClient(newGroupConf)) {
- reply =
- client.getRaftClient().admin().setConfiguration(new
ArrayList<>(newGroupConf.getPeers()));
- if (!reply.isSuccess()) {
+ int basicWaitTime = 500;
+ int maxWaitTime = 10000;
Review Comment:
It feels like we can just check regularly here, without adding idempotent
logic, because it doesn't make a big difference.
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/RegionMaintainHandler.java:
##########
@@ -742,12 +759,27 @@ public void transferRegionLeader(TConsensusGroupId
regionId, TDataNodeLocation o
*/
public Optional<TDataNodeLocation> filterDataNodeWithOtherRegionReplica(
TConsensusGroupId regionId, TDataNodeLocation filterLocation) {
+ List<TDataNodeLocation> filterLocations = new ArrayList<>();
Review Comment:
Collections.singletonList
##########
iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/ratis/RatisConsensus.java:
##########
@@ -869,9 +873,34 @@ private RaftClientReply sendReconfiguration(RaftGroup
newGroupConf)
// notify the group leader of configuration change
RaftClientReply reply;
try (RatisClient client = getRaftClient(newGroupConf)) {
- reply =
- client.getRaftClient().admin().setConfiguration(new
ArrayList<>(newGroupConf.getPeers()));
- if (!reply.isSuccess()) {
+ int basicWaitTime = 500;
+ int maxWaitTime = 10000;
+ int retryTimes = 0;
+ while (true) {
+ logger.info("retry sendConfiguration req {} times", retryTimes);
+ reply =
+ client
+ .getRaftClient()
+ .admin()
+ .setConfiguration(new ArrayList<>(newGroupConf.getPeers()));
+ if (reply.isSuccess()) {
+ logger.info("reConfiguration success");
+ break;
+ }
+ if (reply.getException() instanceof ReconfigurationInProgressException
+ || reply.getException() instanceof LeaderSteppingDownException
Review Comment:
when we need thses two exceptions? will this cause forever loop?
##########
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:
OK, waiting for yuheng's pr
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/RegionMaintainHandler.java:
##########
@@ -667,12 +667,21 @@ public void removeDataNodePersistence(TDataNodeLocation
dataNodeLocation) {
*/
public void transferRegionLeader(TConsensusGroupId regionId,
TDataNodeLocation originalDataNode)
throws ProcedureException, InterruptedException {
+ List<TDataNodeLocation> excludeDataNode =
Collections.singletonList(originalDataNode);
+ transferRegionLeader(regionId, originalDataNode, excludeDataNode);
+ }
+
+ public void transferRegionLeader(
+ TConsensusGroupId regionId,
+ TDataNodeLocation originalDataNode,
+ List<TDataNodeLocation> excludeDataNode)
Review Comment:
seems we only need one node which will be deleted in the future? not a list?
--
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]
