Skip to content

update state without proposal when leader changes in zero #2319

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

Merged
merged 3 commits into from
Apr 12, 2018

Conversation

janardhan1993
Copy link
Contributor

@janardhan1993 janardhan1993 commented Apr 9, 2018

Fixes #2273

This change is Reviewable

@janardhan1993
Copy link
Contributor Author

Review status: 0 of 3 files reviewed at latest revision, all discussions resolved.


dgraph/cmd/zero/raft.go, line 407 at r1 (raw file):

func (n *node) triggerLeaderChange() {
	n.server.triggerLeaderChange()
	n.server.updateZeroLeader()

We can update leader information on each node without proposal. This function is called on all nodes on leader change.


dgraph/cmd/zero/raft.go, line 408 at r1 (raw file):

	n.server.triggerLeaderChange()
	m := &intern.Member{Id: n.Id, Addr: n.RaftContext.Addr, Leader: n.AmLeader()}
	go n.proposeAndWait(context.Background(), &intern.ZeroProposal{Member: m})

Proposing it in goroutine can mess up the order and cause wrong state.


worker/groups.go, line 250 at r1 (raw file):

	g.Lock()
	defer g.Unlock()
	if g.state != nil && g.state.Counter >= state.Counter {

We don't update state if we get any old state. Counter stores the raftindex of last state. For leader changes at zero since we don't propose, state can get updated at same counter value.


Comments from Reviewable

@pawanrawal
Copy link
Contributor

:lgtm: for the first two changes. Add the comments explaining them in the code please.

Not sure why we create another copy of the key. Does ReadPostingList modify the key?


Reviewed 3 of 3 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 3 unresolved discussions.


Comments from Reviewable

@janardhan1993 janardhan1993 merged commit 3673f50 into master Apr 12, 2018
@ghost ghost removed the in progress label Apr 12, 2018
@janardhan1993 janardhan1993 deleted the jan/node_lockup branch April 12, 2018 06:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants