search

goraft / raft

UNMAINTAINED: A Go implementation of the Raft distributed consensus protocol.
MIT License
2.43k stars 479 forks source link

fix(server.go) fix incorrect stateChangeEvent dispatching #213

Closed xiang90 closed 10 years ago

xiang90 commented 10 years ago

Fix a bug introduced by race fix. The setState function will be called even if there is no actual stateChange. Check the previous state before dispatching the event.

xiang90 commented 10 years ago

the bug is introduced here https://github.com/goraft/raft/commit/01e8793c9a031a13f8fcc67784545d76d96e94f9#diff-34c6b408d72845d076d47126c29948d1R906

yichengq commented 10 years ago

I think in general, we should check whether or not s.state == state and return it at very first time. But it seems that the function also takes charge of reporting leader change, so I am unsure about it.

xiang90 commented 10 years ago

@unihorn We can do that, but I would rather make that happen in another refactor pull request.

yichengq commented 10 years ago

But if it doesn't impact the leader change report, we could make do the s.state == state check and return it if false at the start of this function. I think that could make logic easier.

xiang90 commented 10 years ago

@unihorn Updated. How about this?

yichengq commented 10 years ago

@xiangli-cmu Could it be in Snapshotting state when processAppendEntriesRequest ? If so, the if condition should consider about this also.

xiang90 commented 10 years ago

@unihorn Only candidate should step-down to follower. Both follower and snapshotting should be as same as it is.

yichengq commented 10 years ago

@xiangli-cmu lgtm then!