Skip to content

RATIS-2392. Leader should trigger heartbeat immediately after ReadIndex#1340

Open
ivandika3 wants to merge 2 commits intoapache:masterfrom
ivandika3:RATIS-2392
Open

RATIS-2392. Leader should trigger heartbeat immediately after ReadIndex#1340
ivandika3 wants to merge 2 commits intoapache:masterfrom
ivandika3:RATIS-2392

Conversation

@ivandika3
Copy link
Contributor

@ivandika3 ivandika3 commented Feb 4, 2026

What changes were proposed in this pull request?

Please refer to https://issues.apache.org/jira/browse/RATIS-2392

What is the link to the Apache JIRA

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

How was this patch tested?

Manually through TestOzoneShellHAWithFollowerRead (see the RATIS-2392 comment) and existing CI (https://github.com/ivandika3/ratis/actions/runs/21665802316)

Copy link
Contributor

@szetszwo szetszwo left a comment

Choose a reason for hiding this comment

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

@ivandika3 , this is a great finding!

Instead of trigger a heartbeat, how about adding a CommitInfoProto list to ReadIndexReplyProto? Then, the follower can update the commit indices with the reply.

@ivandika3
Copy link
Contributor Author

ivandika3 commented Feb 6, 2026

@szetszwo Thanks for the review.

Instead of trigger a heartbeat, how about adding a CommitInfoProto list to ReadIndexReplyProto. Then, the follower can update the commit indices with the reply.

As specified by the Raft paper, Raft follower updates its commitIndex only when handling AppendEntries from a valid leader. Therefore, I'm not sure whether updating the follower's commitIndex can be done out-of-band (i.e. using CommitInfoProto in another ReadIndex channel) without affecting correctness. Especially there were previous issues where there were race condition issues on heartbeat and append log (e.g. RATIS-2234, RATIS-2235, RATIS-2242, etc).

Let me know if I'm misunderstood.

@szetszwo
Copy link
Contributor

szetszwo commented Feb 6, 2026

... aft follower updates its commitIndex only when handling AppendEntries from a valid leader. ...

That's is a good point! It seems no harm to update commitIndex for readIndex since

  • the follower has initiated the call to the leader so the leader must be valid (if not, the entire readIndex algorithm won't work), and
  • AppendEntries and heartbeat can be separated so updating commitIndex must already support concurrent update (agree that there was a bug RATIS-2234 previously. The other two bugs RATIS-2235, RATIS-2242 are related to AppendEntries but not updating commitIndex.)

... how about adding a CommitInfoProto list to ReadIndexReplyProto? ...

Actually, we only have to add leader's commitIndex but not a CommitInfoProto list.

@ivandika3 , I could see that adding leader's commitIndex to ReadIndexReplyProto needs more changes and makes the code complicated. I am fine if we keep the current approach using triggerHeartbeat(). I respect your decision.

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