Raft stability #2111
Raft stability #2111
Conversation
|
Just did a quick review and LGTM. Although I haven't fully digested it yet, we need to get this in for RC17 so I'll merge and hopefully @otoolep, @corylanou, and @dgnorton can do further review tomorrow. |
| @@ -352,32 +351,32 @@ func copyFileN(w io.Writer, path string, n int64) (int64, error) { | |||
| return io.CopyN(w, f, n) | |||
| } | |||
|
|
|||
| // Restore reads the broker state. | |||
| func (b *Broker) Restore(r io.Reader) error { | |||
| // ReadFrom reads a broker snapshot from r. | |||
There was a problem hiding this comment.
Minor: describe what the returned int64 is?
There was a problem hiding this comment.
It's the number of bytes read but we're not supporting it. It's just an implementation of the io.ReaderFrom interface. We return an error for any partial reads.
| @@ -28,10 +28,6 @@ var ( | |||
| // ErrOutOfDateLog is returned when a candidate's log is not up to date. | |||
| ErrOutOfDateLog = errors.New("out of date log") | |||
|
|
|||
| // ErrUncommittedIndex is returned when a stream is started from an | |||
There was a problem hiding this comment.
This can't happen anymore?
There was a problem hiding this comment.
No, the stream starts from the highest available index on the leader and the follower will trim its log or re-request a snapshot if needed.
| @@ -436,6 +523,12 @@ func (l *Log) writeTerm(term uint64) error { | |||
| return ioutil.WriteFile(l.termPath(), b, 0666) | |||
| } | |||
|
|
|||
| // setTerm sets the current term if it's higher. | |||
There was a problem hiding this comment.
This comment does not look correct. I don't see any check within the code. It just sets the term unconditionally.
|
I reviewed, most of my feedback is minor. |
fix(http): return http 400 if request data is invalid
Overview
This pull request adds several
raftpackage clean up fixes.A list of high level changes:
FSMonly - previously this index was shared between theraft.Logand theFSM.FSMAPI to implementio.WriterToandio.ReaderFrom. The "applied index" refactor above altered the API so conforming to a standard API makes this easier to test.This pull request also documents the various internal fields in
raft.Log.