lni / dragonboat

A feature complete and high performance multi-group Raft library in Go.
Apache License 2.0
4.99k stars 534 forks source link

get leader vote only after leader persisted #224

Closed leisurelyrcxf closed 2 years ago

leisurelyrcxf commented 2 years ago

Currently get leader vote before persisted, which may result in data inconsistent if leader crashes.

E.g., 3 nodes: 1,2,3, leader is 1.

Get vote of 2, and 1 not persisted leader crash-> 2 get voted -> 1 up -> 2 crash -> 1 voted again The system has two live nodes (1, 3), but the committed log is lost.

Or consider the single quorum scenario. In current implementation, it will commit even before log persisted, which is against the WAL semantic.

lni commented 2 years ago

@leisurelyrcxf

I don't quite understand the 3 nodes scenario you described above. Could you please add more details like when logs are appended and what committed logs can be lost.

Please also note that vote related messages won't be sent before logs are persistently stored with fsync(). Please check the engine.go and node.go for details.

For the mentioned single quorum case, yes, the log is marked as committed in memory before being persistently stored, however, a bold however, (1) client is not notified for such commit event before the log is persistently stored, please see engine.go and node.go for details, (2) not any node will be notified for such commit event. I don't think it violates anything.

lni commented 2 years ago

btw, if you can email me your WeChat account name, we can talk on WeChat in more details.

My email address is nilei81@gmail.com

leisurelyrcxf commented 2 years ago

ha. I finally got the logic. if ud.FastApply { if len(ud.CommittedEntries) > 0 && len(ud.EntriesToSave) > 0 { lastApplyIndex := ud.CommittedEntries[len(ud.CommittedEntries)-1].Index lastSaveIndex := ud.EntriesToSave[len(ud.EntriesToSave)-1].Index firstSaveIndex := ud.EntriesToSave[0].Index if lastApplyIndex >= firstSaveIndex && lastApplyIndex <= lastSaveIndex { ud.FastApply = false } } } When see a CommittedEntries before persisted, the check will pass. and FastApply will become false, this prevent apply before persist.

leisurelyrcxf commented 2 years ago

Can close this MR. Thank you!