liyuheng55555 commented on code in PR #13178:
URL: https://github.com/apache/iotdb/pull/13178#discussion_r1719604597
##########
iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/ratis/RatisConsensus.java:
##########
@@ -922,18 +922,18 @@ public void allowStaleRead(ConsensusGroupId
consensusGroupId) {
private class RatisClientPoolFactory implements
IClientPoolFactory<RaftGroup, RatisClient> {
- private boolean isReConfiguration;
+ private final boolean isReconfiguration;
RatisClientPoolFactory(boolean isReConfiguration) {
- this.isReConfiguration = isReConfiguration;
+ this.isReconfiguration = isReConfiguration;
Review Comment:
you forget something
##########
iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/ratis/RatisClient.java:
##########
@@ -212,46 +218,39 @@ public Action handleAttemptFailure(Event event) {
}
// This policy is used to raft configuration change
+ //
Review Comment:
Use javadoc /** */ instead
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/RegionMaintainHandler.java:
##########
@@ -665,27 +665,26 @@ public void removeDataNodePersistence(TDataNodeLocation
dataNodeLocation) {
* @param regionId The region to be migrated
* @param originalDataNode The DataNode where the region locates
*/
- 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)
+ TConsensusGroupId regionId, TDataNodeLocation originalDataNode,
TDataNodeLocation coodinator)
throws ProcedureException, InterruptedException {
// find new leader
final int findNewLeaderTimeLimitSecond = 10;
long startTime = System.nanoTime();
Optional<TDataNodeLocation> newLeaderNode = Optional.empty();
+ List<TDataNodeLocation> excludeDataNode = new ArrayList<>();
+ excludeDataNode.add(originalDataNode);
+ excludeDataNode.add(coodinator);
while (System.nanoTime() - startTime <
TimeUnit.SECONDS.toNanos(findNewLeaderTimeLimitSecond)) {
newLeaderNode = filterDataNodeWithOtherRegionReplica(regionId,
excludeDataNode);
if (newLeaderNode.isPresent()) {
break;
}
}
+ if (!newLeaderNode.isPresent()) {
+ // If we have no choice, we use it
+ newLeaderNode = Optional.of(coodinator);
+ }
Review Comment:
Better than my thought, nice and clean code!
##########
iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/ratis/RatisClient.java:
##########
@@ -212,46 +218,39 @@ public Action handleAttemptFailure(Event event) {
}
// This policy is used to raft configuration change
+ //
private static class RatisEndlessRetryPolicy implements RetryPolicy {
private static final Logger logger =
LoggerFactory.getLogger(RatisEndlessRetryPolicy.class);
- private static final RetryPolicy defaultPolicy;
-
- static {
- String str = "";
- // 50, 500ms, 40, 1000ms, 30, 1500ms, 20, 2000ms, 10, 2500ms
- int basicRetry = 50;
- int basicSleep = 500;
- for (int i = 0; i < 5; i++) {
- str += basicRetry + "," + basicSleep + ",";
- basicRetry -= 10;
- basicSleep += 500;
- }
+ private final RetryPolicy defaultPolicy;
+ RatisEndlessRetryPolicy() {
+ // about 1 hour wait Time.
defaultPolicy =
- MultipleLinearRandomRetry.parseCommaSeparated(str.substring(0,
str.length() - 1));
+ RetryPolicies.retryForeverWithSleep(TimeDuration.valueOf(2,
TimeUnit.SECONDS));
}
Review Comment:
1. Isn't retry every 2 seconds too frequently ?
2. Why still // about 1 hour wait Time.
--
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]
