Skip to content
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

RATIS-1291. Send heartbeat when there is no reply #398

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

RATIS-1291. Send heartbeat when there is no reply #398

wants to merge 1 commit into from

Conversation

runzhiwang
Copy link
Contributor

@runzhiwang runzhiwang commented Jan 20, 2021

What changes were proposed in this pull request?

Currently, when there always exist log, leader only send log, and will not send heartbeat. The problem is if follower need a
long time to process log, leader maybe receives response from follower after a long time.

  default long getHeartbeatRemainingTimeMs() {
    return getServer().properties().minRpcTimeoutMs()/2 - getFollower().getLastRpcTime().elapsedTimeMs();
  }

In this pr, I record lastRpcSendTimeWithResponse, i.e. when leader receive response of request1, leader record the send time of request1. Besides, I record lastHeartBeatSendTime, i.e. the send time of heartbeat.

Use this two timestamp, leader send heartbeat when can not receive response after minRpcTimeoutMs()/2. If log cost a
short time from request to reply, leader also does not need to send heartbeat.

  default long getHeartbeatRemainingTimeMs() {
    return getServer().properties().minRpcTimeoutMs()/2 -
       Math.min(getFollower().getLastRpcSendTimeWithResponse().elapsedTimeMs(),
           getFollower().getLastHeartBeatSendTime().elapsedTimeMs());
  }

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/RATIS-1291

How was this patch tested?

no need new ut

@runzhiwang runzhiwang closed this Jan 20, 2021
@runzhiwang runzhiwang reopened this Jan 20, 2021
@runzhiwang runzhiwang closed this Jan 20, 2021
@runzhiwang runzhiwang reopened this Jan 20, 2021
@runzhiwang
Copy link
Contributor Author

will fix the failed ut.

@runzhiwang runzhiwang closed this Jan 20, 2021
@runzhiwang runzhiwang reopened this Jan 20, 2021
@amaliujia
Copy link
Contributor

cc @amaliujia I can help review when the UT is fixed.

@@ -143,7 +143,6 @@ public void run() throws IOException {
}

appendLog(installSnapshotRequired || haveTooManyPendingRequests());

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: undo this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants