-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Enable FSM batching through optionally implemented interface #364
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work Brian!
A couple of pretty minor thoughts inline but overall this looks great.
I know we recently got fuzzy working again, is is worth adding a fuzz test that exercises batched FSM to get more confidence we haven't violated the FSM invariants in face or concurrent operation ans errors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing!
Separate PR for the existing metric makes sense - would be good to do that soon though so our products can end up with consistent metric types in next release. We should also make sure to document in CHANGELOG for raft and notify our own teams so they can note the change in metric type for their release notes.
Currently this library operates under the assumption that the FSM apply will be much faster than the Log apply. Based on this assumption the library does Log apply batching but no FSM level batching. When using an in-memory FSM this isn't a problem, but when the FSM is doing additional work- such as persisting the data to disk- this causes high latencies.
This PR introduces a new
BatchingFSM
interface which extends theFSM
interface to add anApplyBatch([]*Log) []interface{}
function. This can optionally be implemented by clients to enable multiple logs to be applied to the FSM in batches. Up toMaxAppendEntries
could be sent in a batch.ApplyBatch
is invoked once a batch of log entries has been committed and are ready to be applied to the FSM.ApplyBatch
will take in an array of log entries. These log entries could be of a few types so clients should check the log type prior to attempting to decode the data attached. Presently theLogCommand
andLogConfiguration
types will be sent.The returned slice must be the same length as the input. The returned values will be made available in the
ApplyFuture
returned by raft.Apply method if that method was called on the same Raft node as the FSM.