From 4d2b582fbbdad2bb1e280cc2f33715fdcccd4962 Mon Sep 17 00:00:00 2001
From: Martin Martinez Rivera <martinmr@dgraph.io>
Date: Mon, 8 Jun 2020 11:44:42 -0700
Subject: [PATCH] Avoid assigning duplicate RAFT IDs to new nodes. (#5571)

Currently, zero uses the MaxRaftId to assign RAFT IDs to new nodes. However, the
MaxRaftId value is not immediately updated and the lock is released in between
creating the proposal and sending the proposal to the RAFT node. This can lead
to situations where more than one new node is assigned the same ID.

To solve this, this change introduces a new field to generate the IDs that is
updated immediately, thus preventing multiple nodes from being assigned
the same ID.

Fixes #5436
---
 dgraph/cmd/zero/raft.go |  1 +
 dgraph/cmd/zero/zero.go | 11 ++++++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/dgraph/cmd/zero/raft.go b/dgraph/cmd/zero/raft.go
index 49ad38e866c..168e8e05b63 100644
--- a/dgraph/cmd/zero/raft.go
+++ b/dgraph/cmd/zero/raft.go
@@ -324,6 +324,7 @@ func (n *node) applyProposal(e raftpb.Entry) (string, error) {
 			return p.Key, errInvalidProposal
 		}
 		state.MaxRaftId = p.MaxRaftId
+		n.server.nextRaftId = x.Max(n.server.nextRaftId, p.MaxRaftId+1)
 	}
 	if p.SnapshotTs != nil {
 		for gid, ts := range p.SnapshotTs {
diff --git a/dgraph/cmd/zero/zero.go b/dgraph/cmd/zero/zero.go
index 44e1abf7d80..2c8b69f12fe 100644
--- a/dgraph/cmd/zero/zero.go
+++ b/dgraph/cmd/zero/zero.go
@@ -55,6 +55,7 @@ type Server struct {
 
 	NumReplicas int
 	state       *pb.MembershipState
+	nextRaftId  uint64
 
 	nextLeaseId uint64
 	nextTxnTs   uint64
@@ -82,6 +83,7 @@ func (s *Server) Init() {
 		Groups: make(map[uint32]*pb.Group),
 		Zeros:  make(map[uint64]*pb.Member),
 	}
+	s.nextRaftId = 1
 	s.nextLeaseId = 1
 	s.nextTxnTs = 1
 	s.nextGroup = 1
@@ -215,7 +217,10 @@ func (s *Server) hasLeader(gid uint32) bool {
 func (s *Server) SetMembershipState(state *pb.MembershipState) {
 	s.Lock()
 	defer s.Unlock()
+
 	s.state = state
+	s.nextRaftId = x.Max(s.nextRaftId, s.state.MaxRaftId+1)
+
 	if state.Zeros == nil {
 		state.Zeros = make(map[uint64]*pb.Member)
 	}
@@ -489,7 +494,11 @@ func (s *Server) Connect(ctx context.Context,
 			}
 		}
 		if m.Id == 0 {
-			m.Id = s.state.MaxRaftId + 1
+			// In certain situations, the proposal can be sent and return with an error.
+			// However,  Dgraph will keep retrying the proposal. To avoid assigning duplicating
+			// IDs, the couter is incremented every time a proposal is created.
+			m.Id = s.nextRaftId
+			s.nextRaftId += 1
 			proposal.MaxRaftId = m.Id
 		}