Merge pull request #10864 from tbg/learner-snap

raft: allow voter to become learner through snapshot

raft/raft.go

@@ -1374,16 +1374,6 @@ func (r *raft) restore(s pb.Snapshot) bool {
 		return false
 	}
 
-	// The normal peer can't become learner.
-	if !r.isLearner {
-		for _, id := range s.Metadata.ConfState.Learners {
-			if id == r.id {
-				r.logger.Errorf("%x can't become learner when restores snapshot [index: %d, term: %d]", r.id, s.Metadata.Index, s.Metadata.Term)
-				return false
-			}
-		}
-	}
-
 	r.raftLog.restore(s)
 
 	// Reset the configuration and add the (potentially updated) peers in anew.

raft/raft_test.go

@@ -2777,9 +2777,15 @@ func TestRestoreWithLearner(t *testing.T) {
 	}
 }
 
-// TestRestoreInvalidLearner verfies that a normal peer can't become learner again
+// TestRestoreVoterToLearner verifies that a normal peer can be downgraded to a
-// when restores snapshot.
+// learner through a snapshot. At the time of writing, we don't allow
-func TestRestoreInvalidLearner(t *testing.T) {
+// configuration changes to do this directly, but note that the snapshot may
+// compress multiple changes to the configuration into one: the voter could have
+// been removed, then readded as a learner and the snapshot reflects both
+// changes. In that case, a voter receives a snapshot telling it that it is now
+// a learner. In fact, the node has to accept that snapshot, or it is
+// permanently cut off from the Raft log.
+func TestRestoreVoterToLearner(t *testing.T) {
 	s := pb.Snapshot{
 		Metadata: pb.SnapshotMetadata{
 			Index:     11, // magic number
@@ -2794,8 +2800,8 @@ func TestRestoreInvalidLearner(t *testing.T) {
 	if sm.isLearner {
 		t.Errorf("%x is learner, want not", sm.id)
 	}
-	if ok := sm.restore(s); ok {
+	if ok := sm.restore(s); !ok {
-		t.Error("restore succeed, want fail")
+		t.Error("restore failed unexpectedly")
 	}
 }