[WIP] Prevent unbounded Raft log growth. #139
Conversation
utsl42
force-pushed
the
bounded-raft-log
branch
from
761a726 to
9fa860e
Compare
| /// determines whether they would push leader over its max_uncommitted_entries_size limit. | ||
| /// If the new entries would exceed the limit, the method returns false. If not, | ||
| /// the increase in uncommitted entry size is recorded and the method returns | ||
| /// true. |
There was a problem hiding this comment.
This needs a # Panics section which details that passing 0 as max will cause a panic.
There was a problem hiding this comment.
I only added the panic to help me chase down an annoying bug where I broke all the tests. Can just pull it out.
| @@ -252,6 +261,13 @@ impl<T: Storage> RaftLog<T> { | |||
| self.last_index() | |||
| ) | |||
| } | |||
| let entries = self | |||
| .slice( | |||
There was a problem hiding this comment.
Could this use unstable_entries instead of slice? If so, the unwrap would become unnecessary here.
There was a problem hiding this comment.
I worry in this situation that there would be entries written to stable storage, but not yet committed.
You can see here: https://docs.rs/raft/0.4.0/raft/raw_node/struct.Ready.html#structfield.committed_entries
CommittedEntries specifies entries to be committed to a store/state-machine. These have previously been committed to stable store.
Meanwhile: https://docs.rs/raft/0.4.0/raft/raw_node/struct.Ready.html#structfield.entries
Entries specifies entries to be saved to stable storage BEFORE Messages are sent.
So if we only rely on unstable_entries I think it means we might not see all the uncommitted entries.
|
Hi @utsl42! Are you still working on this? |
|
Have had to pause for a bit, but I do plan to pick it back up later, likely
in January. At this point, I think it mostly just needs tests.
…On Thu, Nov 29, 2018 at 1:40 PM A. Hobden ***@***.***> wrote:
Hi @utsl42 <https://github.com/utsl42>! Are you still working on this?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#139 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ADs1Ff5r7xeIj_KNBdAgw9HTM4isr5hZks5u0CoWgaJpZM4YVFPg>
.
|
|
Hi @utsl42, still thinking of picking this back up? |
Hoverbear
added
the
Help Wanted
|
@Hoverbear @utsl42 do you want to finish it by youself or can i try to help you? |
|
@652h If you wanted to open up a revival PR I'd be very happy to help shepherd it to merge! I was hoping to revisit this soon but haven't have a moment. |
|
|
|
This was implemented as #398, close it now, thanks! |
Issue #131