You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ratis.apache.org by "Elia-Renzoni (via GitHub)" <gi...@apache.org> on 2026/09/12 16:39:32 UTC
[PR] RATIS-2311. Control appliedIndex before starting a transfer election [ratis]
Elia-Renzoni opened a new pull request, #1586:
URL: https://github.com/apache/ratis/pull/1586
## What changes were proposed in this pull request?
This PR makes the leadership transfer check in TransferLeadership#isFollowerUpToDate stricter by also verifying that the gap between the target follower's matchIndex (replicated log) and its appliedIndex (state machine apply progress) is within an acceptable threshold, in addition to the existing matchIndex up-to-date check
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/RATIS-2311?page=com.atlassian.jira.plugin.system.issuetabpanels%3Aall-tabpanel
## How was this patch tested?
Added ```testRiskyLeadershipTransfer``` to the abstract ```LeaderElectionTests``` class, alongside the existing ```testTransferLeader``` and ```testTransferLeaderTimeout``` 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
Re: [PR] RATIS-2311. Control appliedIndex before starting a transfer election [ratis]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo merged PR #1586:
URL: https://github.com/apache/ratis/pull/1586
--
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
Re: [PR] RATIS-2311. Control appliedIndex before starting a transfer election [ratis]
Posted by "Elia-Renzoni (via GitHub)" <gi...@apache.org>.
Elia-Renzoni commented on code in PR #1586:
URL: https://github.com/apache/ratis/pull/1586#discussion_r4156029526
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/TransferLeadership.java:
##########
@@ -79,12 +79,14 @@ enum Type {
TIMED_OUT,
FAILED_TO_START,
COMPLETED_EXCEPTIONALLY,
+ RISKY_LEADER_CHANGE,
}
static final Result SUCCESS = new Result(Type.SUCCESS);
static final Result DIFFERENT_LEADER = new Result(Type.DIFFERENT_LEADER);
static final Result NULL_FOLLOWER = new Result(Type.NULL_FOLLOWER);
static final Result NULL_LOG_APPENDER = new Result(Type.NULL_LOG_APPENDER);
+ static final Result RISKY_LEADER_CHANGE = new Result(Type.RISKY_LEADER_CHANGE);
Review Comment:
Fixed here: 110c11ab35d96ecdd87c967e0fac998b14aa9925
--
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
Re: [PR] RATIS-2311. Control appliedIndex before starting a transfer election [ratis]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #1586:
URL: https://github.com/apache/ratis/pull/1586#issuecomment-5880971659
I looked at this once. Since this PR adds new public API and conf, I need some more time to think about the changes.
--
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
Re: [PR] RATIS-2311. Control appliedIndex before starting a transfer election [ratis]
Posted by "Elia-Renzoni (via GitHub)" <gi...@apache.org>.
Elia-Renzoni commented on code in PR #1586:
URL: https://github.com/apache/ratis/pull/1586#discussion_r4156032028
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/TransferLeadership.java:
##########
@@ -164,12 +166,16 @@ public String toString() {
private final RaftServerImpl server;
private final TimeDuration requestTimeout;
private final TimeoutExecutor scheduler = TimeoutExecutor.getInstance();
+ private final long appliedIndexThreshold;
private final AtomicReference<PendingRequest> pending = new AtomicReference<>();
TransferLeadership(RaftServerImpl server, RaftProperties properties) {
this.server = server;
this.requestTimeout = RaftServerConfigKeys.Rpc.requestTimeout(properties);
+ this.appliedIndexThreshold = RaftServerConfigKeys
+ .LeaderElection
+ .transferLeadershipRequiredAppliedIndexGap(properties);
Review Comment:
Fixed here: 110c11ab35d96ecdd87c967e0fac998b14aa9925
##########
ratis-server/src/main/java/org/apache/ratis/server/leader/LogAppenderDefault.java:
##########
@@ -204,6 +205,7 @@ void handleReply(AppendEntriesReplyProto reply, long requestFirstIndex,
+ ", reply=" + ServerStringUtils.toAppendEntriesReplyString(reply));
}
+ getFollower().updateAppliedIndex(followerAppliedIndex);
Review Comment:
Fixed here: 110c11ab35d96ecdd87c967e0fac998b14aa9925
--
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
Re: [PR] RATIS-2311. Control appliedIndex before starting a transfer election [ratis]
Posted by "Elia-Renzoni (via GitHub)" <gi...@apache.org>.
Elia-Renzoni commented on PR #1586:
URL: https://github.com/apache/ratis/pull/1586#issuecomment-5791258535
@szetszwo Could you take a look at this PR? Thanks!
--
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
Re: [PR] RATIS-2311. Control appliedIndex before starting a transfer election [ratis]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #1586:
URL: https://github.com/apache/ratis/pull/1586#discussion_r4137191120
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/TransferLeadership.java:
##########
@@ -79,12 +79,14 @@ enum Type {
TIMED_OUT,
FAILED_TO_START,
COMPLETED_EXCEPTIONALLY,
+ RISKY_LEADER_CHANGE,
}
static final Result SUCCESS = new Result(Type.SUCCESS);
static final Result DIFFERENT_LEADER = new Result(Type.DIFFERENT_LEADER);
static final Result NULL_FOLLOWER = new Result(Type.NULL_FOLLOWER);
static final Result NULL_LOG_APPENDER = new Result(Type.NULL_LOG_APPENDER);
+ static final Result RISKY_LEADER_CHANGE = new Result(Type.RISKY_LEADER_CHANGE);
Review Comment:
This is unused. Let's remove it.
##########
ratis-server/src/main/java/org/apache/ratis/server/leader/LogAppenderDefault.java:
##########
@@ -204,6 +205,7 @@ void handleReply(AppendEntriesReplyProto reply, long requestFirstIndex,
+ ", reply=" + ServerStringUtils.toAppendEntriesReplyString(reply));
}
+ getFollower().updateAppliedIndex(followerAppliedIndex);
Review Comment:
Call getFollowerAppliedIndex() here and remove followerAppliedIndex:
```
getFollower().updateAppliedIndex(reply.getFollowerAppliedIndex());
```
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/TransferLeadership.java:
##########
@@ -79,12 +79,14 @@ enum Type {
TIMED_OUT,
FAILED_TO_START,
COMPLETED_EXCEPTIONALLY,
+ RISKY_LEADER_CHANGE,
Review Comment:
Let's call it `FAILED_APPLIED_INDEX_GAP`.
##########
ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java:
##########
@@ -1031,6 +1031,21 @@ static boolean memberMajorityAdd(RaftProperties properties) {
static void setMemberMajorityAdd(RaftProperties properties, boolean enableMemberMajorityAdd) {
setBoolean(properties::setBoolean, MEMBER_MAJORITY_ADD_KEY, enableMemberMajorityAdd);
}
+
+ String TRANSFER_LEADERSHIP_REQUIRED_APPLIED_INDEX_GAP_KEY = ".transfer-leadership.required-applied-index-gap";
+ long TRANSFER_LEADERSHIP_REQUIRED_APPLIED_INDEX_GAP_DEFAULT = 2_000L;
+ static long transferLeadershipRequiredAppliedIndexGap(RaftProperties properties) {
+ return getLong(
+ properties::getLong,
+ TRANSFER_LEADERSHIP_REQUIRED_APPLIED_INDEX_GAP_KEY,
+ TRANSFER_LEADERSHIP_REQUIRED_APPLIED_INDEX_GAP_DEFAULT,
+ getDefaultLog(),
+ requireMin(0L)
+ );
+ }
+ static void setTransferLeadershipRequiredAppliedIndexGap(RaftProperties properties, long appliedIndexValue) {
+ setLong(properties::setLong, TRANSFER_LEADERSHIP_REQUIRED_APPLIED_INDEX_GAP_KEY, appliedIndexValue);
+ }
Review Comment:
- Need PREFIX.
- Let's rename it to ".leader.transfer.applied-index.gap" and move it right below ".leader.step-down.wait-time". They are kind of related.
```java
String LEADER_TRANSFER_APPLIED_INDEX_GAP_KEY = PREFIX + ".leader.transfer.applied-index.gap";
long LEADER_TRANSFER_APPLIED_INDEX_GAP_DEFAULT = STAGING_CATCHUP_GAP_DEFAULT;
static long leaderTransferAppliedIndexGap(RaftProperties properties) {
return getLong(properties::getLong,
LEADER_TRANSFER_APPLIED_INDEX_GAP_KEY,
LEADER_TRANSFER_APPLIED_INDEX_GAP_DEFAULT,
getDefaultLog(), requireMin(0L)
);
}
static void setLeaderTransferAppliedIndexGap(RaftProperties properties, long appliedIndexValue) {
setLong(properties::setLong, LEADER_TRANSFER_APPLIED_INDEX_GAP_KEY, appliedIndexValue);
}
```
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/TransferLeadership.java:
##########
@@ -164,12 +166,16 @@ public String toString() {
private final RaftServerImpl server;
private final TimeDuration requestTimeout;
private final TimeoutExecutor scheduler = TimeoutExecutor.getInstance();
+ private final long appliedIndexThreshold;
private final AtomicReference<PendingRequest> pending = new AtomicReference<>();
TransferLeadership(RaftServerImpl server, RaftProperties properties) {
this.server = server;
this.requestTimeout = RaftServerConfigKeys.Rpc.requestTimeout(properties);
+ this.appliedIndexThreshold = RaftServerConfigKeys
+ .LeaderElection
+ .transferLeadershipRequiredAppliedIndexGap(properties);
Review Comment:
Use single line:
```java
this.appliedIndexThreshold = RaftServerConfigKeys.LeaderElection.leaderTransferAppliedIndexGap(properties);
```
--
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
Re: [PR] RATIS-2311. Control appliedIndex before starting a transfer election [ratis]
Posted by "Elia-Renzoni (via GitHub)" <gi...@apache.org>.
Elia-Renzoni commented on code in PR #1586:
URL: https://github.com/apache/ratis/pull/1586#discussion_r4156026415
##########
ratis-server-api/src/main/java/org/apache/ratis/server/RaftServerConfigKeys.java:
##########
@@ -1031,6 +1031,21 @@ static boolean memberMajorityAdd(RaftProperties properties) {
static void setMemberMajorityAdd(RaftProperties properties, boolean enableMemberMajorityAdd) {
setBoolean(properties::setBoolean, MEMBER_MAJORITY_ADD_KEY, enableMemberMajorityAdd);
}
+
+ String TRANSFER_LEADERSHIP_REQUIRED_APPLIED_INDEX_GAP_KEY = ".transfer-leadership.required-applied-index-gap";
+ long TRANSFER_LEADERSHIP_REQUIRED_APPLIED_INDEX_GAP_DEFAULT = 2_000L;
+ static long transferLeadershipRequiredAppliedIndexGap(RaftProperties properties) {
+ return getLong(
+ properties::getLong,
+ TRANSFER_LEADERSHIP_REQUIRED_APPLIED_INDEX_GAP_KEY,
+ TRANSFER_LEADERSHIP_REQUIRED_APPLIED_INDEX_GAP_DEFAULT,
+ getDefaultLog(),
+ requireMin(0L)
+ );
+ }
+ static void setTransferLeadershipRequiredAppliedIndexGap(RaftProperties properties, long appliedIndexValue) {
+ setLong(properties::setLong, TRANSFER_LEADERSHIP_REQUIRED_APPLIED_INDEX_GAP_KEY, appliedIndexValue);
+ }
Review Comment:
Addressed here: 110c11ab35d96ecdd87c967e0fac998b14aa9925
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/TransferLeadership.java:
##########
@@ -79,12 +79,14 @@ enum Type {
TIMED_OUT,
FAILED_TO_START,
COMPLETED_EXCEPTIONALLY,
+ RISKY_LEADER_CHANGE,
Review Comment:
Fixed here: 110c11ab35d96ecdd87c967e0fac998b14aa9925
--
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
Re: [PR] RATIS-2311. Control appliedIndex before starting a transfer election [ratis]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #1586:
URL: https://github.com/apache/ratis/pull/1586#issuecomment-5935976316
> [Elia-Renzoni](https://github.com/Elia-Renzoni) [force-pushed](https://github.com/apache/ratis/compare/7a9f350482d90e3e1a785901f5650206704458f6..110c11ab35d96ecdd87c967e0fac998b14aa9925) the refactor/transfer-leadership branch from [7a9f350](https://github.com/apache/ratis/commit/7a9f350482d90e3e1a785901f5650206704458f6) to [110c11a](https://github.com/apache/ratis/commit/110c11ab35d96ecdd87c967e0fac998b14aa9925)
@Elia-Renzoni , please don't force push after a review has started. Otherwise, it also forces the reviewers to read everything again since it is hard to see only the new changes.
--
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
Re: [PR] RATIS-2311. Control appliedIndex before starting a transfer election [ratis]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #1586:
URL: https://github.com/apache/ratis/pull/1586#issuecomment-5805005348
Sure, will review this.
--
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