search

zhebrak / raftos

Asynchronous replication framework for distributed Python projects
MIT License
350 stars 58 forks source link

Change in update commit index #30

Closed meteam2022 closed 1 year ago

meteam2022 commented 1 year ago

This pull request addresses the issue described in #26

close #26

meteam2022 commented 1 year ago

Hi @zhebrak,

I would like to apologize for my oversight in this pull request. After careful review, I realized that the changes I proposed here unintentionally addressed a different bug, while leaving the originally linked issue (#26) unresolved. I sincerely apologize for any confusion caused.

The root cause of the issue addressed in this pull request is as follows:

Describe the Bug

Due to the requirements of the Raft protocol, it is not allowed to commit a log entry that does not belong to the current term. In the code implementation, there is a loop used to update the commit index, which iterates sequentially. However, the loop breaks when encountering an index that does not belong to the current term. As a result, subsequent indices belonging to the current term and eligible for commit are not being checked, causing the commit index to remain stagnant.

https://github.com/zhebrak/raftos/blob/0d6f9e049b526279b1035f597291a96cf50c9b40/raftos/state.py#L240-L255

To Reproduce

Scenario:

  1. A becomes the leader (term=1).
  2. A receives a client request and appends a log entry with index=1 and term=1 to the log. It sends an AppendEntries message to B.
  3. B receives the AppendEntries message and replies with an AppendEntriesResponse. However, due to network delay or packet loss, A has not received this message yet.
  4. B initiates an election and becomes the leader (term=2). As per RaftOS, A transitions to a follower upon receiving the first RequestVote. It votes for B upon receiving the second duplicate RequestVote.
  5. B receives a client request, appends a log entry with index=2 and term=2 to the log, and sends an AppendEntries message to A. A responds with an AppendEntriesResponse. Upon receiving the response, B should update the commit index to 2. However, due to the bug described above, B fails to update the commit index.
  6. Repeating step 5 does not resolve the issue; the commit index remains unchanged.

raftos-30

Since the pull request has been merged, there is no need for any further actions on it. I apologize for any confusion caused.

Best regards