search

NicolasT / kontiki

An implementation of the Raft consensus protocol
BSD 3-Clause "New" or "Revised" License
122 stars 15 forks source link

No commit information in API? #7

Closed qrilka closed 10 years ago

qrilka commented 11 years ago

It looks like handle functions transform AppendEntries into Command and thus all the information about committed entries in Raft log gets lost. Am I right or have I missed something? As far as I understand to get correctly committed entries is the main purpose of the protocol.

NicolasT commented 11 years ago

Uh, damn, you're right. That's a major (and rather stupid) oversight.

Looks like the Command type should get another constructor used to indicate the current commit index and allow the application-level to update its internal state accordingly.

Would you care to draft up a patch?

qrilka commented 11 years ago

@NicolasT am I right that current implementation is not quite correct that Leader updates commitIndex only on EHeartbeatTimeout and not in handleAppendEntriesResponse as it should do? If I'm right then I propose the following: Looking into Raft paper in protocol summary (on page 4) I see leaderCommit in AppendEntries and it seems quite reasonable to me to use the same method. For Followers we could split AppendEntries into CLogEntries and CSetCommitIndex. And Leader will get CSetCommitIndex as result from handleAppendEntriesResponse. Does it seem reasonable?

NicolasT commented 11 years ago

Indeed: a CSetCommitIndex constructor should be added to Command, and this should be issued from handleAppendEntriesResponse when in Leader state, or from handleAppendEntries when in Follower state, using the commitIndex found in the message.