search

tikv / raft-rs

Raft distributed consensus algorithm implemented in Rust.
Apache License 2.0
2.86k stars 391 forks source link

allow raft apply committed logs before they are persisted #537

Closed glorv closed 3 months ago

glorv commented 4 months ago

ref: tikv/tikv#16457, https://github.com/tikv/tikv/issues/16717

Design doc: https://github.com/tikv/rfcs/pull/112

As committed means more than quorum node are persisted, which no data loss even if all other node are down. So in this situation, it is safe to apply this log even if it is still not persisted.

This PR introduces a new config max_applied_unpersisted_log_limit that allows return unpersisted raft log in light_ready. This is one step to optimize the tail latency that one slow node can significantly impact the overall latency. By setting a proper value for max_applied_unpersisted_log_limit we can avoid the in memory raft entries consumes too much memory which may lead to OOM.

After this change, if max_applied_unpersisted_log_limit is > 0, then it is possible: 1) applied > persisted. 2) applied > committed.(Only can happen at restart). So we loose some check since they are not always true anymore.

glorv commented 4 months ago

@Connor1996 @gengliqi @overvenus @tonyxuqqi @BusyJay PTAL, thank you~

cfzjywxk commented 4 months ago

Perhaps we could organize and link a top-down design document or RFC to illustrate the specific changes in tikv/raftstore, interface impacts, etc., after removing the implicit code constraints related to apply entry, as well as the corresponding design in raft-rs.

I remember @gengliqi has some relevant design documents before?

tonyxuqqi commented 3 months ago

@glorv We should have a RFC document to summarize the design.

glorv commented 3 months ago

@Connor1996 @gengliqi PTAL again, thanks~

gengliqi commented 3 months ago

I think we need to add more restrictions to these logs that can be applied before persisting so that correctness errors can be found. My idea of the restriction is: A log can be applied before persisting only when a log with the same term has been committed and persisted. After restarting, we can check if the logs between committed and applied have the same term as the last committed entry. If not the same, there must be something wrong and the panic must happen immediately.

gengliqi commented 3 months ago

I will propose another pr to implement this restriction.