Closed xiang90 closed 11 years ago
@xiangli-cmu Sorry for the delay in getting to this. Why is this in the Raft library? Sync isn't a part of the Raft protocol. Can you move this to etcd?
@benbjohnson I want to add this into raft library for the following reason:
@benbjohnson Also this add no overhead to raft, if people do not want to use sync.
Sync is a strange concept in Raft since Raft is meant to deterministically change a state machine. A sync shouldn't be necessary in most applications. Even if there is a sync in another application, the details will probably be different.
Adding it may not add CPU/network overhead but it complicates the library. Can you move it over to etcd and we'll integrate it back into Raft if we see a lot of people trying to do something similar? The etcd PeerServer
already knows about Raft. I don't think it's too bad to have it manage the sync based on the leader state.
@benbjohnson OK I will have a try. Currently I can see we need to add a mechanism to push the status change to etcd in raft. I do not want the sync based on a polling model, since most of the nodes will be followers.
@xiangli-cmu I'm ok if you want to include some kind of callback from the Server
for state changes. Maybe something like:
type StateChangeHandler interface {
HandleStateChange(*Server)
}
And then add a function to Server
it:
type Server interface {
...
SetStateChangeHandler(StateChangeHandler)
}
Just make sure it's called synchronously on state change. No channels or goroutines.
@benbjohnson I will use a polling model first. It all the ttl approach works well, i will make the change at raft side.
add sync functionality