Browse Source

raft: Fix a nil-pointer panic in MultiNode.Propose.

Ben Darnell 10 years ago
parent
commit
05924b330a
2 changed files with 24 additions and 4 deletions
  1. 4 2
      raft/multinode.go
  2. 20 2
      raft/multinode_test.go

+ 4 - 2
raft/multinode.go

@@ -227,8 +227,10 @@ func (mn *multiNode) run() {
 			// We'll have to buffer somewhere on a group-by-group basis, or just let
 			// We'll have to buffer somewhere on a group-by-group basis, or just let
 			// raft.Step drop any such proposals on the floor.
 			// raft.Step drop any such proposals on the floor.
 			mm.msg.From = mn.id
 			mm.msg.From = mn.id
-			group = groups[mm.group]
-			group.raft.Step(mm.msg)
+			var ok bool
+			if group, ok = groups[mm.group]; ok {
+				group.raft.Step(mm.msg)
+			}
 
 
 		case mm := <-mn.recvc:
 		case mm := <-mn.recvc:
 			group = groups[mm.group]
 			group = groups[mm.group]

+ 20 - 2
raft/multinode_test.go

@@ -206,8 +206,26 @@ func TestMultiNodeProposeConfig(t *testing.T) {
 	}
 	}
 }
 }
 
 
-// TestBlockProposal from node_test.go has no equivalent in multiNode
-// because we cannot block proposals based on individual group leader status.
+// TestProposeUnknownGroup ensures that we gracefully handle proposals
+// for groups we don't know about (which can happen on a former leader
+// that has been removed from the group).
+//
+// It is analogous to TestBlockProposal from node_test.go but in
+// MultiNode we cannot block proposals based on individual group
+// leader status.
+func TestProposeUnknownGroup(t *testing.T) {
+	mn := newMultiNode(1)
+	go mn.run()
+	defer mn.Stop()
+
+	// A nil error from Propose() doesn't mean much. In this case the
+	// proposal will be dropped on the floor because we don't know
+	// anything about group 42. This is a very crude test that mainly
+	// guarantees that we don't panic in this case.
+	if err := mn.Propose(context.TODO(), 42, []byte("somedata")); err != nil {
+		t.Errorf("err = %v, want nil", err)
+	}
+}
 
 
 // TestNodeTick from node_test.go has no equivalent in multiNode because
 // TestNodeTick from node_test.go has no equivalent in multiNode because
 // it reaches into the raft object which is not exposed.
 // it reaches into the raft object which is not exposed.