RATIS-2392. Leader should trigger heartbeat immediately after ReadIndex

[ivandika3]

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)

[szetszwo]

@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]

@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]

... 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.

[OneSizeFitsQuorum]

Good idea!
+1 for current method. Triggering the heartbeat is a low-risk, high-reward fix: it’s simple to implement and provides the necessary performance without regressing the codebase.

[ivandika3]

Thanks @szetszwo for the explanation and @OneSizeFitsQuorum for the review.

I think we can go ahead with the current approach first, I'll benchmark @szetszwo approach and will raise a ticket if there are improvements and there is no regression.

FYI, @szetszwo patch might look like read-index-leader-commit.patch (Edit: looks like the CI pass https://github.com/ivandika3/ratis/actions/runs/21810574780)

[szetszwo]

+1 the change looks good.

[szetszwo]

Thanks @ivandika3 for working on this!

Thanks @OneSizeFitsQuorum for reviewing this!