Re: [PR] RATIS-2392. Leader should trigger heartbeat immediately after ReadIndex [ratis]
szetszwo commented on PR #1340: URL: https://github.com/apache/ratis/pull/1340#issuecomment-3869103836 Thanks @ivandika3 for working on this! Thanks @OneSizeFitsQuorum for reviewing 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] RATIS-2392. Leader should trigger heartbeat immediately after ReadIndex [ratis]
szetszwo merged PR #1340: URL: https://github.com/apache/ratis/pull/1340 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] RATIS-2392. Leader should trigger heartbeat immediately after ReadIndex [ratis]
ivandika3 commented on PR #1340: URL: https://github.com/apache/ratis/pull/1340#issuecomment-3869020556 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 an improvement. FYI, @szetszwo patch might look like [read-index-leader-commit.patch](https://github.com/user-attachments/files/25172883/read-index-leader-commit.patch) -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] RATIS-2392. Leader should trigger heartbeat immediately after ReadIndex [ratis]
szetszwo commented on PR #1340: URL: https://github.com/apache/ratis/pull/1340#issuecomment-3861797749 > ... 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](https://issues.apache.org/jira/browse/RATIS-2235), [RATIS-2242](https://issues.apache.org/jira/browse/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. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] RATIS-2392. Leader should trigger heartbeat immediately after ReadIndex [ratis]
ivandika3 commented on PR #1340: URL: https://github.com/apache/ratis/pull/1340#issuecomment-3858235230 @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 through 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). 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). -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
