raft: fix read index request for #7331 #7332
Conversation
| @@ -823,6 +823,11 @@ func stepLeader(r *raft, m pb.Message) { | |||
| return | |||
| case pb.MsgReadIndex: | |||
| if r.quorum() > 1 { | |||
| if r.raftLog.zeroTermOnErrCompacted(r.raftLog.term(r.raftLog.committed)) != r.Term { | |||
| // Reject read only request when this leader has not committed any log entry at its term. | |||
There was a problem hiding this comment.
if we drop the message here directly, how do we know it outside?
There was a problem hiding this comment.
It's usual to drop message like this. E.g. Requests like proposes are usually dropped in raft. And it's possible the a MsgReadIndex is lost due to network issue when follower forwards it to leader.
There should be a timeout/retry of requests outside this module.
Codecov Report
@@ Coverage Diff @@
## master #7332 +/- ##
========================================
Coverage ? 64.9%
========================================
Files ? 230
Lines ? 20723
Branches ? 0
========================================
Hits ? 13450
Misses ? 6281
Partials ? 992
Continue to review full report at Codecov.
|
|
@hhkbp2 Good fix. Can you add a test for it? |
|
@xiang90 A test case is added. |
raft/raft_test.go
Outdated
| // Force peer a to become leader. | ||
| nt.send(pb.Message{From: 1, To: 1, Type: pb.MsgHup}) | ||
| if a.state != StateLeader { | ||
| t.Fatalf("state = %, want %s", a.state, StateLeader) |
|
One minor issue. LGTM after fixing it. |
|
@hhkbp2 Thank you! |
Hi,
This PR tries to fix issue #7331 . It follows the raft thesis 6.4 to reject read only request when a new leader has not committed any log entry at its term.