-
Notifications
You must be signed in to change notification settings - Fork 94
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
IGNITE-15705 election timeout autoadjusting mechanism #481
Conversation
bd11072
to
26a4e1e
Compare
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/NodeImpl.java
Outdated
Show resolved
Hide resolved
modules/raft/src/integrationTest/java/org/apache/ignite/raft/jraft/core/ItNodeTest.java
Show resolved
Hide resolved
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/NodeImpl.java
Outdated
Show resolved
Hide resolved
modules/raft/src/main/java/org/apache/ignite/raft/jraft/option/NodeOptions.java
Outdated
Show resolved
Hide resolved
26a4e1e
to
a2f1b38
Compare
a812547
to
647824b
Compare
modules/raft/src/integrationTest/java/org/apache/ignite/raft/jraft/core/ItNodeTest.java
Outdated
Show resolved
Hide resolved
modules/raft/src/main/java/org/apache/ignite/raft/jraft/util/NoopTimeoutStrategy.java
Show resolved
Hide resolved
modules/raft/src/main/java/org/apache/ignite/internal/raft/server/impl/JraftServerImpl.java
Outdated
Show resolved
Hide resolved
modules/raft/src/main/java/org/apache/ignite/internal/raft/server/impl/JraftServerImpl.java
Outdated
Show resolved
Hide resolved
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/NodeImpl.java
Outdated
Show resolved
Hide resolved
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/NodeImpl.java
Outdated
Show resolved
Hide resolved
.../raft/src/main/java/org/apache/ignite/raft/jraft/util/ExponentialBackoffTimeoutStrategy.java
Show resolved
Hide resolved
modules/raft/src/main/java/org/apache/ignite/internal/raft/server/impl/JraftServerImpl.java
Outdated
Show resolved
Hide resolved
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/NodeImpl.java
Outdated
Show resolved
Hide resolved
09246fb
to
c3bf6d6
Compare
modules/raft/src/main/java/org/apache/ignite/internal/raft/server/impl/JraftServerImpl.java
Outdated
Show resolved
Hide resolved
modules/raft/src/main/java/org/apache/ignite/internal/raft/server/impl/JraftServerImpl.java
Outdated
Show resolved
Hide resolved
2d8ba3f
to
5dbf8c1
Compare
5dbf8c1
to
59ce843
Compare
private static final int DEFAULT_TIMEOUT_MS_MAX = 11_000; | ||
|
||
/** Default max number of a round after which timeout will be adjusted. */ | ||
public static final long DEFAULT_ROUNDS_WITHOUT_ADJUSTING = 3; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this public ?
private long roundsWithoutAdjusting = DEFAULT_ROUNDS_WITHOUT_ADJUSTING; | ||
|
||
public ExponentialBackoffTimeoutStrategy() { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
call to this with defaults is missing
long timeout = options.getElectionTimeoutStrategy().nextTimeout(options.getElectionTimeoutMs(), electionRound); | ||
|
||
if (timeout != options.getElectionTimeoutMs()) { | ||
LOG.info("Election timeout was adjusted according to {} ", options.getElectionTimeoutStrategy().toString()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
toString is not needed
LOG.info("Election timeout was adjusted according to {} ", options.getElectionTimeoutStrategy().toString()); | ||
resetElectionTimeoutMs((int) timeout); | ||
electionAdjusted = true; | ||
electionRound = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we reset electionRound to zero after each succesful adjustment ?
It doesn't seem correct to me. Additionally, I would log this value on each election attempt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought that we could use the logic with several election attempts without adjusting also after the moment when the timeout was adjusted. Motivation is to postpone a moment when we reach the maximum election timeout.
Seems, that logic without resetting is more clear, I'll revert this change.
https://issues.apache.org/jira/browse/IGNITE-15705