-
Notifications
You must be signed in to change notification settings - Fork 399
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
raft: always keep un-applied logs in memory #199
Comments
In TiKV, we use a cache outside to avoid fetching logs from Storage. But if we can do it in raft lib directly, maybe it is more common. Here the think we need to care is the memory usage of this cache, we can't save a lot to avoid OOM. |
Avoiding OOM is exactly what I'm considering about. I notice there is a #131 which describes a protection mechanism for the leader to prevent unlimited raft logs growth and it inspires me a little. An intuitive approach to solve the issue for me is to have some configurations for limitation e.g. When the follower's in-memory raft logs reach the memory limit, it should send a msg (e.g For a detailed design of leader throttle:
The leader throttle approach might be somewhat aggressive here. For a succinct solution, we can just use configures. And if in-memory raft logs reach the limit, the /cc @Hoverbear |
This can be implemented in storage instead of the raft library. It's better to keep the library simple. |
@Fullstop000 Maybe you and I could make a new more sophisticated storage module crate for Raft that does this? :) |
@Hoverbear if you have any idea to improve this, I'd like to help with it. |
@Fullstop000 Do you think we could come up with a simple file based storage for demonstration purposes? Or maybe use something like SLED? |
@Hoverbear I prefer that we start this with file-based storage from scratch so that we can later easily add some best practices about how to use the raft lib. For now, the two examples might not be enough for some 'real-world' situations. |
That sounds great, @Fullstop000 . :) Do you think you can take care of an initial PR and I will make myself available to help how I can? |
@Hoverbear somewhat busy recently 😢. Maybe I could commit an initial PR for some ideas about the Storage on mid-June |
In the current version of implementation, if we want to apply committed entries, we will try to retrieve them from
raft_log
first. At that time, it's very likely that the entries we want are stored in theStorage
, which might be insufficient to get them ( especially when theStorage
is based on something like rocksDB).Generally, the issue described above often occurs when the updating speed of
committed_index
is not as fast as raft log growing so I think it's a common situation we need to improve.Therefore, we can always keep un-applied logs in memory ( the
unstable
) even after we store them into theStorage
.The text was updated successfully, but these errors were encountered: