willemt / raft

C implementation of the Raft Consensus protocol, BSD licensed
Other
1.13k stars 271 forks source link

Not implemented log_clear callback leads to not synchronized logs #108

Open langchr86 opened 4 years ago

langchr86 commented 4 years ago

You mentioned some of the callbacks that need to be implemented in the README. Sadly in our case we must also implement the log_clear callback for the case we load a snapshot with log_load_from_snapshot. Because in this case the whole log should be cleared (is empty afterwards) before applying the snapshot. Initially I thought that this will be done by using the callbacks log_pop and/or log_poll but it isn't. Only log_clear is implicitly called by using log_clear_entries internally of log_load_from_snapshot.

In our case we still had 1 entry in our own log where the raft internal log was empty after loading a snapshot.

Either this is a bug in the documentation (because this callback is needed if working with callbacks) or the implementation of log_load_from_snapshot should handle the case where the callback is not available.

In addition the description of the callback is not very clear to me:

    /** Callback called for every existing log entry when clearing the log.
     * If memory was malloc'd in log_offer and the entry doesn't get a chance
     * to go through log_poll or log_pop, this is the last chance to free it.
     */

I do not see the requirement to individually handle single items. In my implementation I simply clear my whole log at once. I am using an std::queue with smart enough element class that manages its internal memory. Therefore I simply call: log_.clear();.