You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ratis.apache.org by GitBox <gi...@apache.org> on 2022/08/16 02:53:35 UTC
[GitHub] [ratis] SzyWilliam opened a new pull request, #713: RATIS-1638. Separate first election timeout
SzyWilliam opened a new pull request, #713:
URL: https://github.com/apache/ratis/pull/713
## What changes were proposed in this pull request?
ElectionTimeout cannot be set too small, which will lead to frequent leader change when full GC occurs. However, longer ElectionTimeout will cause longer unavailable times during start up, which is annoying.
In this PR I propose to separate first election timeout and normal election timeout. This can enable Raft Group elect a leader quickly during start up while avoid frequent leader change during normal operations.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/RATIS-1638
## How was this patch tested?
unit tests
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] SzyWilliam commented on a diff in pull request #713: RATIS-1638. Separate first election timeout
Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on code in PR #713:
URL: https://github.com/apache/ratis/pull/713#discussion_r949759755
##########
ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java:
##########
@@ -608,24 +608,54 @@ interface Rpc {
String TIMEOUT_MIN_KEY = PREFIX + ".timeout.min";
TimeDuration TIMEOUT_MIN_DEFAULT = TimeDuration.valueOf(150, TimeUnit.MILLISECONDS);
- static TimeDuration timeoutMin(RaftProperties properties) {
+ static TimeDuration timeoutMin(RaftProperties properties, Consumer<String> logger) {
return getTimeDuration(properties.getTimeDuration(TIMEOUT_MIN_DEFAULT.getUnit()),
- TIMEOUT_MIN_KEY, TIMEOUT_MIN_DEFAULT, getDefaultLog());
+ TIMEOUT_MIN_KEY, TIMEOUT_MIN_DEFAULT, logger);
+ }
+ static TimeDuration timeoutMin(RaftProperties properties) {
+ return timeoutMin(properties, getDefaultLog());
}
static void setTimeoutMin(RaftProperties properties, TimeDuration minDuration) {
setTimeDuration(properties::setTimeDuration, TIMEOUT_MIN_KEY, minDuration);
}
String TIMEOUT_MAX_KEY = PREFIX + ".timeout.max";
TimeDuration TIMEOUT_MAX_DEFAULT = TimeDuration.valueOf(300, TimeUnit.MILLISECONDS);
- static TimeDuration timeoutMax(RaftProperties properties) {
+ static TimeDuration timeoutMax(RaftProperties properties, Consumer<String> logger) {
return getTimeDuration(properties.getTimeDuration(TIMEOUT_MAX_DEFAULT.getUnit()),
- TIMEOUT_MAX_KEY, TIMEOUT_MAX_DEFAULT, getDefaultLog());
+ TIMEOUT_MAX_KEY, TIMEOUT_MAX_DEFAULT, logger);
+ }
+ static TimeDuration timeoutMax(RaftProperties properties) {
+ return timeoutMax(properties, getDefaultLog());
}
static void setTimeoutMax(RaftProperties properties, TimeDuration maxDuration) {
setTimeDuration(properties::setTimeDuration, TIMEOUT_MAX_KEY, maxDuration);
}
+ /** separate first timeout so that the startup unavailable time can be reduced */
+ String FIRST_ELECTION_TIMEOUT_MIN_KEY = PREFIX + ".first-election.timeout.min";
+ TimeDuration FIRST_ELECTION_TIMEOUT_MIN_DEFAULT = null;
+ static TimeDuration firstElectionTimeoutMin(RaftProperties properties) {
+ final TimeDuration fallbackFirstElectionTimeoutMin = Rpc.timeoutMin(properties, null);
+ return getTimeDuration(properties.getTimeDuration(fallbackFirstElectionTimeoutMin.getUnit()),
+ FIRST_ELECTION_TIMEOUT_MIN_KEY, FIRST_ELECTION_TIMEOUT_MIN_DEFAULT,
+ Rpc.TIMEOUT_MIN_KEY, fallbackFirstElectionTimeoutMin, getDefaultLog());
+ }
+ static void setFirstElectionTimeoutMin(RaftProperties properties, TimeDuration firstMinDuration) {
+ setTimeDuration(properties::setTimeDuration, FIRST_ELECTION_TIMEOUT_MIN_KEY, firstMinDuration);
+ }
+
+ String FIRST_ELECTION_TIMEOUT_MAX_KEY = PREFIX + ".first-election.timeout.max";
+ TimeDuration FIRST_ELECTION_TIMEOUT_MAX_DEFAULT = null;
+ static TimeDuration firstElectionTimeoutMax(RaftProperties properties) {
+ final TimeDuration fallbackFirstElectionTimeoutMax = Rpc.timeoutMax(properties, null);
+ return getTimeDuration(properties.getTimeDuration(fallbackFirstElectionTimeoutMax.getUnit()),
+ FIRST_ELECTION_TIMEOUT_MAX_KEY, fallbackFirstElectionTimeoutMax, getDefaultLog());
Review Comment:
done
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] SzyWilliam commented on a diff in pull request #713: RATIS-1638. Separate first election timeout
Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on code in PR #713:
URL: https://github.com/apache/ratis/pull/713#discussion_r949121483
##########
ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java:
##########
@@ -626,6 +626,29 @@ static void setTimeoutMax(RaftProperties properties, TimeDuration maxDuration) {
setTimeDuration(properties::setTimeDuration, TIMEOUT_MAX_KEY, maxDuration);
}
+ /** separate first timeout so that the startup unavailable time can be reduced */
+ String FIRST_TIMEOUT_MIN_KEY = PREFIX + ".first.timeout.min";
+ TimeDuration FIRST_TIMEOUT_MIN_DEFAULT = TimeDuration.valueOf(150, TimeUnit.MILLISECONDS);
+ static TimeDuration firstTimeoutMin(RaftProperties properties) {
+ final TimeDuration fallbackFirstTimeoutMin = Rpc.timeoutMin(properties);
+ return getTimeDuration(properties.getTimeDuration(fallbackFirstTimeoutMin.getUnit()),
Review Comment:
done
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] SzyWilliam commented on a diff in pull request #713: RATIS-1638. Separate first election timeout
Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on code in PR #713:
URL: https://github.com/apache/ratis/pull/713#discussion_r949810462
##########
ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java:
##########
@@ -608,24 +608,55 @@ interface Rpc {
String TIMEOUT_MIN_KEY = PREFIX + ".timeout.min";
TimeDuration TIMEOUT_MIN_DEFAULT = TimeDuration.valueOf(150, TimeUnit.MILLISECONDS);
- static TimeDuration timeoutMin(RaftProperties properties) {
+ static TimeDuration timeoutMin(RaftProperties properties, Consumer<String> logger) {
return getTimeDuration(properties.getTimeDuration(TIMEOUT_MIN_DEFAULT.getUnit()),
- TIMEOUT_MIN_KEY, TIMEOUT_MIN_DEFAULT, getDefaultLog());
+ TIMEOUT_MIN_KEY, TIMEOUT_MIN_DEFAULT, logger);
+ }
+ static TimeDuration timeoutMin(RaftProperties properties) {
+ return timeoutMin(properties, getDefaultLog());
}
static void setTimeoutMin(RaftProperties properties, TimeDuration minDuration) {
setTimeDuration(properties::setTimeDuration, TIMEOUT_MIN_KEY, minDuration);
}
String TIMEOUT_MAX_KEY = PREFIX + ".timeout.max";
TimeDuration TIMEOUT_MAX_DEFAULT = TimeDuration.valueOf(300, TimeUnit.MILLISECONDS);
- static TimeDuration timeoutMax(RaftProperties properties) {
+ static TimeDuration timeoutMax(RaftProperties properties, Consumer<String> logger) {
return getTimeDuration(properties.getTimeDuration(TIMEOUT_MAX_DEFAULT.getUnit()),
- TIMEOUT_MAX_KEY, TIMEOUT_MAX_DEFAULT, getDefaultLog());
+ TIMEOUT_MAX_KEY, TIMEOUT_MAX_DEFAULT, logger);
+ }
+ static TimeDuration timeoutMax(RaftProperties properties) {
+ return timeoutMax(properties, getDefaultLog());
}
static void setTimeoutMax(RaftProperties properties, TimeDuration maxDuration) {
setTimeDuration(properties::setTimeDuration, TIMEOUT_MAX_KEY, maxDuration);
}
+ /** separate first timeout so that the startup unavailable time can be reduced */
+ String FIRST_ELECTION_TIMEOUT_MIN_KEY = PREFIX + ".first-election.timeout.min";
+ TimeDuration FIRST_ELECTION_TIMEOUT_MIN_DEFAULT = null;
+ static TimeDuration firstElectionTimeoutMin(RaftProperties properties) {
+ final TimeDuration fallbackFirstElectionTimeoutMin = Rpc.timeoutMin(properties, null);
+ return getTimeDuration(properties.getTimeDuration(fallbackFirstElectionTimeoutMin.getUnit()),
+ FIRST_ELECTION_TIMEOUT_MIN_KEY, FIRST_ELECTION_TIMEOUT_MIN_DEFAULT,
+ Rpc.TIMEOUT_MIN_KEY, fallbackFirstElectionTimeoutMin, getDefaultLog());
+ }
+ static void setFirstElectionTimeoutMin(RaftProperties properties, TimeDuration firstMinDuration) {
+ setTimeDuration(properties::setTimeDuration, FIRST_ELECTION_TIMEOUT_MIN_KEY, firstMinDuration);
+ }
+
+ String FIRST_ELECTION_TIMEOUT_MAX_KEY = PREFIX + ".first-election.timeout.max";
+ TimeDuration FIRST_ELECTION_TIMEOUT_MAX_DEFAULT = null;
+ static TimeDuration firstElectionTimeoutMax(RaftProperties properties) {
+ final TimeDuration fallbackFirstElectionTimeoutMax = Rpc.timeoutMax(properties, null);
+ return getTimeDuration(properties.getTimeDuration(fallbackFirstElectionTimeoutMax.getUnit()),
+ FIRST_ELECTION_TIMEOUT_MAX_KEY, TIMEOUT_MAX_DEFAULT,
Review Comment:
Done. Thanks for the careful review.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on a diff in pull request #713: RATIS-1638. Separate first election timeout
Posted by GitBox <gi...@apache.org>.
szetszwo commented on code in PR #713:
URL: https://github.com/apache/ratis/pull/713#discussion_r949788717
##########
ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java:
##########
@@ -608,24 +608,55 @@ interface Rpc {
String TIMEOUT_MIN_KEY = PREFIX + ".timeout.min";
TimeDuration TIMEOUT_MIN_DEFAULT = TimeDuration.valueOf(150, TimeUnit.MILLISECONDS);
- static TimeDuration timeoutMin(RaftProperties properties) {
+ static TimeDuration timeoutMin(RaftProperties properties, Consumer<String> logger) {
return getTimeDuration(properties.getTimeDuration(TIMEOUT_MIN_DEFAULT.getUnit()),
- TIMEOUT_MIN_KEY, TIMEOUT_MIN_DEFAULT, getDefaultLog());
+ TIMEOUT_MIN_KEY, TIMEOUT_MIN_DEFAULT, logger);
+ }
+ static TimeDuration timeoutMin(RaftProperties properties) {
+ return timeoutMin(properties, getDefaultLog());
}
static void setTimeoutMin(RaftProperties properties, TimeDuration minDuration) {
setTimeDuration(properties::setTimeDuration, TIMEOUT_MIN_KEY, minDuration);
}
String TIMEOUT_MAX_KEY = PREFIX + ".timeout.max";
TimeDuration TIMEOUT_MAX_DEFAULT = TimeDuration.valueOf(300, TimeUnit.MILLISECONDS);
- static TimeDuration timeoutMax(RaftProperties properties) {
+ static TimeDuration timeoutMax(RaftProperties properties, Consumer<String> logger) {
return getTimeDuration(properties.getTimeDuration(TIMEOUT_MAX_DEFAULT.getUnit()),
- TIMEOUT_MAX_KEY, TIMEOUT_MAX_DEFAULT, getDefaultLog());
+ TIMEOUT_MAX_KEY, TIMEOUT_MAX_DEFAULT, logger);
+ }
+ static TimeDuration timeoutMax(RaftProperties properties) {
+ return timeoutMax(properties, getDefaultLog());
}
static void setTimeoutMax(RaftProperties properties, TimeDuration maxDuration) {
setTimeDuration(properties::setTimeDuration, TIMEOUT_MAX_KEY, maxDuration);
}
+ /** separate first timeout so that the startup unavailable time can be reduced */
+ String FIRST_ELECTION_TIMEOUT_MIN_KEY = PREFIX + ".first-election.timeout.min";
+ TimeDuration FIRST_ELECTION_TIMEOUT_MIN_DEFAULT = null;
+ static TimeDuration firstElectionTimeoutMin(RaftProperties properties) {
+ final TimeDuration fallbackFirstElectionTimeoutMin = Rpc.timeoutMin(properties, null);
+ return getTimeDuration(properties.getTimeDuration(fallbackFirstElectionTimeoutMin.getUnit()),
+ FIRST_ELECTION_TIMEOUT_MIN_KEY, FIRST_ELECTION_TIMEOUT_MIN_DEFAULT,
+ Rpc.TIMEOUT_MIN_KEY, fallbackFirstElectionTimeoutMin, getDefaultLog());
+ }
+ static void setFirstElectionTimeoutMin(RaftProperties properties, TimeDuration firstMinDuration) {
+ setTimeDuration(properties::setTimeDuration, FIRST_ELECTION_TIMEOUT_MIN_KEY, firstMinDuration);
+ }
+
+ String FIRST_ELECTION_TIMEOUT_MAX_KEY = PREFIX + ".first-election.timeout.max";
+ TimeDuration FIRST_ELECTION_TIMEOUT_MAX_DEFAULT = null;
+ static TimeDuration firstElectionTimeoutMax(RaftProperties properties) {
+ final TimeDuration fallbackFirstElectionTimeoutMax = Rpc.timeoutMax(properties, null);
+ return getTimeDuration(properties.getTimeDuration(fallbackFirstElectionTimeoutMax.getUnit()),
+ FIRST_ELECTION_TIMEOUT_MAX_KEY, TIMEOUT_MAX_DEFAULT,
Review Comment:
TIMEOUT_MAX_DEFAULT should be FIRST_ELECTION_TIMEOUT_MAX_DEFAULT, i.e.
```
FIRST_ELECTION_TIMEOUT_MAX_KEY, FIRST_ELECTION_TIMEOUT_MAX_DEFAULT,
```
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] SzyWilliam commented on pull request #713: RATIS-1638. Separate first election timeout
Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on PR #713:
URL: https://github.com/apache/ratis/pull/713#issuecomment-1220190784
@szetszwo Thanks for the review. I've include the fallback bugfix in this change.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on a diff in pull request #713: RATIS-1638. Separate first election timeout
Posted by GitBox <gi...@apache.org>.
szetszwo commented on code in PR #713:
URL: https://github.com/apache/ratis/pull/713#discussion_r949389163
##########
ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java:
##########
@@ -608,24 +608,54 @@ interface Rpc {
String TIMEOUT_MIN_KEY = PREFIX + ".timeout.min";
TimeDuration TIMEOUT_MIN_DEFAULT = TimeDuration.valueOf(150, TimeUnit.MILLISECONDS);
- static TimeDuration timeoutMin(RaftProperties properties) {
+ static TimeDuration timeoutMin(RaftProperties properties, Consumer<String> logger) {
return getTimeDuration(properties.getTimeDuration(TIMEOUT_MIN_DEFAULT.getUnit()),
- TIMEOUT_MIN_KEY, TIMEOUT_MIN_DEFAULT, getDefaultLog());
+ TIMEOUT_MIN_KEY, TIMEOUT_MIN_DEFAULT, logger);
+ }
+ static TimeDuration timeoutMin(RaftProperties properties) {
+ return timeoutMin(properties, getDefaultLog());
}
static void setTimeoutMin(RaftProperties properties, TimeDuration minDuration) {
setTimeDuration(properties::setTimeDuration, TIMEOUT_MIN_KEY, minDuration);
}
String TIMEOUT_MAX_KEY = PREFIX + ".timeout.max";
TimeDuration TIMEOUT_MAX_DEFAULT = TimeDuration.valueOf(300, TimeUnit.MILLISECONDS);
- static TimeDuration timeoutMax(RaftProperties properties) {
+ static TimeDuration timeoutMax(RaftProperties properties, Consumer<String> logger) {
return getTimeDuration(properties.getTimeDuration(TIMEOUT_MAX_DEFAULT.getUnit()),
- TIMEOUT_MAX_KEY, TIMEOUT_MAX_DEFAULT, getDefaultLog());
+ TIMEOUT_MAX_KEY, TIMEOUT_MAX_DEFAULT, logger);
+ }
+ static TimeDuration timeoutMax(RaftProperties properties) {
+ return timeoutMax(properties, getDefaultLog());
}
static void setTimeoutMax(RaftProperties properties, TimeDuration maxDuration) {
setTimeDuration(properties::setTimeDuration, TIMEOUT_MAX_KEY, maxDuration);
}
+ /** separate first timeout so that the startup unavailable time can be reduced */
+ String FIRST_ELECTION_TIMEOUT_MIN_KEY = PREFIX + ".first-election.timeout.min";
+ TimeDuration FIRST_ELECTION_TIMEOUT_MIN_DEFAULT = null;
+ static TimeDuration firstElectionTimeoutMin(RaftProperties properties) {
+ final TimeDuration fallbackFirstElectionTimeoutMin = Rpc.timeoutMin(properties, null);
+ return getTimeDuration(properties.getTimeDuration(fallbackFirstElectionTimeoutMin.getUnit()),
+ FIRST_ELECTION_TIMEOUT_MIN_KEY, FIRST_ELECTION_TIMEOUT_MIN_DEFAULT,
+ Rpc.TIMEOUT_MIN_KEY, fallbackFirstElectionTimeoutMin, getDefaultLog());
+ }
+ static void setFirstElectionTimeoutMin(RaftProperties properties, TimeDuration firstMinDuration) {
+ setTimeDuration(properties::setTimeDuration, FIRST_ELECTION_TIMEOUT_MIN_KEY, firstMinDuration);
+ }
+
+ String FIRST_ELECTION_TIMEOUT_MAX_KEY = PREFIX + ".first-election.timeout.max";
+ TimeDuration FIRST_ELECTION_TIMEOUT_MAX_DEFAULT = null;
+ static TimeDuration firstElectionTimeoutMax(RaftProperties properties) {
+ final TimeDuration fallbackFirstElectionTimeoutMax = Rpc.timeoutMax(properties, null);
+ return getTimeDuration(properties.getTimeDuration(fallbackFirstElectionTimeoutMax.getUnit()),
+ FIRST_ELECTION_TIMEOUT_MAX_KEY, fallbackFirstElectionTimeoutMax, getDefaultLog());
Review Comment:
It should use the fallback method and in the min.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] SzyWilliam commented on pull request #713: RATIS-1638. Separate first election timeout
Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on PR #713:
URL: https://github.com/apache/ratis/pull/713#issuecomment-1216379366
@codings-dan Thanks for the review! Here's how I solve these two problem:
I add a callback in `setLeader` to disable firstElectionTimeout whenever server has a known leader. That means,
1. The `firstElectionTimeout` is set to false when a new group leader is successfully elected.
2. The `firstElectionTimeout` in two followers is set to false since Leader's AppendEntries will trigger `setLeader` method.
What do you think?
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] SzyWilliam commented on a diff in pull request #713: RATIS-1638. Separate first election timeout
Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on code in PR #713:
URL: https://github.com/apache/ratis/pull/713#discussion_r949125834
##########
ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java:
##########
@@ -626,6 +626,29 @@ static void setTimeoutMax(RaftProperties properties, TimeDuration maxDuration) {
setTimeDuration(properties::setTimeDuration, TIMEOUT_MAX_KEY, maxDuration);
}
+ /** separate first timeout so that the startup unavailable time can be reduced */
+ String FIRST_TIMEOUT_MIN_KEY = PREFIX + ".first.timeout.min";
Review Comment:
done
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo merged pull request #713: RATIS-1638. Separate first election timeout
Posted by GitBox <gi...@apache.org>.
szetszwo merged PR #713:
URL: https://github.com/apache/ratis/pull/713
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] SzyWilliam commented on a diff in pull request #713: RATIS-1638. Separate first election timeout
Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on code in PR #713:
URL: https://github.com/apache/ratis/pull/713#discussion_r947399680
##########
ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java:
##########
@@ -626,6 +626,29 @@ static void setTimeoutMax(RaftProperties properties, TimeDuration maxDuration) {
setTimeDuration(properties::setTimeDuration, TIMEOUT_MAX_KEY, maxDuration);
}
+ /** separate first timeout so that the startup unavailable time can be reduced */
+ String FIRST_TIMEOUT_MIN_KEY = PREFIX + ".first.timeout.min";
+ TimeDuration FIRST_TIMEOUT_MIN_DEFAULT = null;
Review Comment:
Thanks for reminding. I've set these two values corresponding to normal timeout default.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on a diff in pull request #713: RATIS-1638. Separate first election timeout
Posted by GitBox <gi...@apache.org>.
szetszwo commented on code in PR #713:
URL: https://github.com/apache/ratis/pull/713#discussion_r948776745
##########
ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java:
##########
@@ -626,6 +626,29 @@ static void setTimeoutMax(RaftProperties properties, TimeDuration maxDuration) {
setTimeDuration(properties::setTimeDuration, TIMEOUT_MAX_KEY, maxDuration);
}
+ /** separate first timeout so that the startup unavailable time can be reduced */
+ String FIRST_TIMEOUT_MIN_KEY = PREFIX + ".first.timeout.min";
Review Comment:
Let's call it `raft.server.rpc.first-election.timeout.min`.
##########
ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java:
##########
@@ -626,6 +626,29 @@ static void setTimeoutMax(RaftProperties properties, TimeDuration maxDuration) {
setTimeDuration(properties::setTimeDuration, TIMEOUT_MAX_KEY, maxDuration);
}
+ /** separate first timeout so that the startup unavailable time can be reduced */
+ String FIRST_TIMEOUT_MIN_KEY = PREFIX + ".first.timeout.min";
+ TimeDuration FIRST_TIMEOUT_MIN_DEFAULT = TimeDuration.valueOf(150, TimeUnit.MILLISECONDS);
Review Comment:
Let's use null as the default.
##########
ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java:
##########
@@ -626,6 +626,29 @@ static void setTimeoutMax(RaftProperties properties, TimeDuration maxDuration) {
setTimeDuration(properties::setTimeDuration, TIMEOUT_MAX_KEY, maxDuration);
}
+ /** separate first timeout so that the startup unavailable time can be reduced */
+ String FIRST_TIMEOUT_MIN_KEY = PREFIX + ".first.timeout.min";
+ TimeDuration FIRST_TIMEOUT_MIN_DEFAULT = TimeDuration.valueOf(150, TimeUnit.MILLISECONDS);
+ static TimeDuration firstTimeoutMin(RaftProperties properties) {
+ final TimeDuration fallbackFirstTimeoutMin = Rpc.timeoutMin(properties);
Review Comment:
For fallback, we need to pass null for the log so that it won't print an additional log message.
See https://github.com/apache/ratis/blob/323bd1017afdbe03f70cccb172412e6fab221797/ratis-grpc/src/main/java/org/apache/ratis/grpc/GrpcConfigKeys.java#L103
##########
ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java:
##########
@@ -626,6 +626,29 @@ static void setTimeoutMax(RaftProperties properties, TimeDuration maxDuration) {
setTimeDuration(properties::setTimeDuration, TIMEOUT_MAX_KEY, maxDuration);
}
+ /** separate first timeout so that the startup unavailable time can be reduced */
+ String FIRST_TIMEOUT_MIN_KEY = PREFIX + ".first.timeout.min";
+ TimeDuration FIRST_TIMEOUT_MIN_DEFAULT = TimeDuration.valueOf(150, TimeUnit.MILLISECONDS);
+ static TimeDuration firstTimeoutMin(RaftProperties properties) {
+ final TimeDuration fallbackFirstTimeoutMin = Rpc.timeoutMin(properties);
+ return getTimeDuration(properties.getTimeDuration(fallbackFirstTimeoutMin.getUnit()),
Review Comment:
We also need a different get function; see https://github.com/apache/ratis/blob/323bd1017afdbe03f70cccb172412e6fab221797/ratis-grpc/src/main/java/org/apache/ratis/grpc/GrpcConfigKeys.java#L104
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] codings-dan commented on a diff in pull request #713: RATIS-1638. Separate first election timeout
Posted by GitBox <gi...@apache.org>.
codings-dan commented on code in PR #713:
URL: https://github.com/apache/ratis/pull/713#discussion_r947395421
##########
ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java:
##########
@@ -626,6 +626,29 @@ static void setTimeoutMax(RaftProperties properties, TimeDuration maxDuration) {
setTimeDuration(properties::setTimeDuration, TIMEOUT_MAX_KEY, maxDuration);
}
+ /** separate first timeout so that the startup unavailable time can be reduced */
+ String FIRST_TIMEOUT_MIN_KEY = PREFIX + ".first.timeout.min";
+ TimeDuration FIRST_TIMEOUT_MIN_DEFAULT = null;
Review Comment:
We should set a default value, otherwise may cause NPE.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] SzyWilliam commented on pull request #713: RATIS-1638. Separate first election timeout
Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on PR #713:
URL: https://github.com/apache/ratis/pull/713#issuecomment-1219494581
@szetszwo Thanks for the reviews, I've made the corresponding changes. A little question:
the `fallbackKey` and `fallbackValue` are simply printed in `ConfUtil.java`, they are not actually used to substitute RaftProperties KV pair, see https://github.com/apache/ratis/blob/323bd1017afdbe03f70cccb172412e6fab221797/ratis-common/src/main/java/org/apache/ratis/conf/ConfUtils.java#L233. In that case, what's the meaning of using fallbackKey or fallbackValue?
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] SzyWilliam commented on a diff in pull request #713: RATIS-1638. Separate first election timeout
Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on code in PR #713:
URL: https://github.com/apache/ratis/pull/713#discussion_r949121225
##########
ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java:
##########
@@ -626,6 +626,29 @@ static void setTimeoutMax(RaftProperties properties, TimeDuration maxDuration) {
setTimeDuration(properties::setTimeDuration, TIMEOUT_MAX_KEY, maxDuration);
}
+ /** separate first timeout so that the startup unavailable time can be reduced */
+ String FIRST_TIMEOUT_MIN_KEY = PREFIX + ".first.timeout.min";
+ TimeDuration FIRST_TIMEOUT_MIN_DEFAULT = TimeDuration.valueOf(150, TimeUnit.MILLISECONDS);
Review Comment:
done
##########
ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java:
##########
@@ -626,6 +626,29 @@ static void setTimeoutMax(RaftProperties properties, TimeDuration maxDuration) {
setTimeDuration(properties::setTimeDuration, TIMEOUT_MAX_KEY, maxDuration);
}
+ /** separate first timeout so that the startup unavailable time can be reduced */
+ String FIRST_TIMEOUT_MIN_KEY = PREFIX + ".first.timeout.min";
+ TimeDuration FIRST_TIMEOUT_MIN_DEFAULT = TimeDuration.valueOf(150, TimeUnit.MILLISECONDS);
+ static TimeDuration firstTimeoutMin(RaftProperties properties) {
+ final TimeDuration fallbackFirstTimeoutMin = Rpc.timeoutMin(properties);
Review Comment:
done
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on pull request #713: RATIS-1638. Separate first election timeout
Posted by GitBox <gi...@apache.org>.
szetszwo commented on PR #713:
URL: https://github.com/apache/ratis/pull/713#issuecomment-1219728245
> . In that case, what's the meaning of using fallbackKey or fallbackValue?
@SzyWilliam , Good catch! It is a bug. It should be
```java
@@ -236,10 +247,11 @@ public interface ConfUtils {
T value = get(getter, key, defaultValue, null, assertions);
if (value != defaultValue) {
logGet(key, value, defaultValue, logger);
+ return value;
} else {
logFallback(key, fallbackKey, fallbackValue, logger);
+ return fallbackValue;
}
- return value;
}
```
Could you include it in this change?
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org