goraft / raft

UNMAINTAINED: A Go implementation of the Raft distributed consensus protocol.
MIT License
2.43k stars 480 forks source link

feat(server.go) introduce server.Init() #198

Closed xiang90 closed 10 years ago

xiang90 commented 10 years ago

This commit enable dividing raft server starting process to two phase. So application can do extra work based on the last status of the raft server before actually start it. To implement this, we add a initialization phase, in which raft server reads in the log entires from data-dir and commits to the last recorded index.

xiang90 commented 10 years ago

@benbjohnson We need to add this to fix etcd bootstrap problems. We want to be able to do some bootstrap work (send join requests, test the liveness of the previous member) based on its previous state before actually start raft server.
/cc @unihorn

benbjohnson commented 10 years ago

@xiangli-cmu Does the Init() function need to be exported? Are you going to call it directly or are you still calling Start()?

xiang90 commented 10 years ago

@benbjohnson I want to call Init first. Then raft will load all the existing logs. So we are able to examine the state of the statemachine before start the raft server.

yichengq commented 10 years ago

Excited to see the patch! The state of raft changes in this way now: Stopped -> Initialized -> Stopped -> Follower/Candidate/Leader It is kind of weird to see 2 Stopped which contain different meanings.

xiang90 commented 10 years ago

I do not quite follow your comments. Can you try to elaborate that?

yichengq commented 10 years ago

When NewServer, the state of server is Stopped. And it needs to Init first, which makes its state become Initialized. And it starts running now, becomes Follower/Candidate/Leader. When it is stopped on some events, it becomes Stopped. So there exist two types of Stopped, but they have different meanings. I think we should add another stage such as New/Uninitialized.

xiang90 commented 10 years ago

@unihorn Reasonable.

benbjohnson commented 10 years ago

@unihorn Is there something you want to do differently with a New/Uninitialized state? The Server state machine is starting to get unwieldily and I don't want to add states for the sake of adding states.

yichengq commented 10 years ago

@benbjohnson I think the most important thing is to keep the logic of raft clear. But maybe we could integrate Init into NewServer. Ideas? @xiangli-cmu

benbjohnson commented 10 years ago

@unihorn I'd like to stray from the Raft protocol as little as possible. I don't see the point in splitting off an additional state.

xiang90 commented 10 years ago

/cc @benbjohnson @unihorn updated.

yichengq commented 10 years ago

lgtm to me then. Could you also review this? @benbjohnson I really want to bump it into etcd and fix the test error.

benbjohnson commented 10 years ago

Fix the missing error check and then lgtm.