Mukul Kumar Singh commented on RATIS-205:

Thanks for working on this [~szetszwo]. The patch looks really good to me. 
Please find my comments below.

1) CommitInfoCache, both the update functions are similar, can we call one 
function from the other to avoid duplication
2) Also how about we propoulate the cache 
3) FollowerInfo.java, I feel that the new commit index in updateCommitIndex, 
should always be greater than old commit index. Should we add a check for that 
as well ?
4) Raft.proto, should we move the index for commitInfos in 
AppendEntriesRequestProto to 7 ? Same for commitInfos in RaftClientReplyProto.
5) RaftServerImpl.java:342, Can you please add a comment that self state is 
added here and the rest of the values are added later.
6) RaftServerImpl:345, Currently the commit info cache is updated here, Can we 
update it at Logappender.java:234 inside updateCommitIndex ?

> Return commit information to client
> -----------------------------------
>                 Key: RATIS-205
>                 URL: https://issues.apache.org/jira/browse/RATIS-205
>             Project: Ratis
>          Issue Type: Sub-task
>            Reporter: Tsz Wo Nicholas Sze
>            Assignee: Tsz Wo Nicholas Sze
>            Priority: Major
>         Attachments: r205_20180205.patch
> The commit infos should be included in RaftClientReply so that client can get 
> commit infos from the responses of client requests or getInfo.

This message was sent by Atlassian JIRA

Reply via email to