drop Starting cluster state

The special case of Starting allowed us to make sure every node in a
cluster waited for the whole cluster to come up, but caused problems
later if a node died and came back. We drop the Starting state for
clusters, treating a STARTING node as equivalent to an UNKNOWN (or
DOWN) node for purposes of cluster state, so clusters will go from
Down to Degraded to Normal as nodes come up. We now wait for the
Normal state during initial bringup. We would previously have accepted
Degraded, if you could reach it, for instance if a node came up and
then went down again before another node finished starting, but I'm
pretty sure that was unintentional.

This solves a problem where while a node was down, we'd accept
queries that we could handle in a degraded state, but then we'd
*stop* accepting them when the node started coming back up.

api.go

@@ -118,8 +118,7 @@ func (api *API) SetAPIOptions(opts ...apiOption) error {
 // validAPIMethods specifies the api methods that are valid for each
 // cluster state.
 var validAPIMethods = map[disco.ClusterState]map[apiMethod]struct{}{
-	disco.ClusterStateStarting: methodsCommon,
-	disco.ClusterStateNormal:   appendMap(methodsCommon, methodsNormal),
+	disco.ClusterStateNormal: appendMap(methodsCommon, methodsNormal),
 	// Ideally, this would be just `appendMap(methodsCommon, methodsDegraded)`,
 	// but in an attempt to reduce the influence that state (determined by etcd)
 	// has on a node under load, this is set to effectively allow all requests

disco/disco.go

@@ -54,7 +54,6 @@ const (
 	InitialClusterStateExisting InitialClusterState = "existing"
 
 	ClusterStateUnknown  ClusterState = "UNKNOWN"  // default cluster state. It is returned when we are not able to get the real actual state.
-	ClusterStateStarting ClusterState = "STARTING" // cluster is starting and some internal services are not ready yet.
 	ClusterStateDegraded ClusterState = "DEGRADED" // cluster is running but we've lost some # of hosts >0 but < replicaN. Only read queries are allowed.
 	ClusterStateNormal   ClusterState = "NORMAL"   // cluster is up and running.
 	ClusterStateDown     ClusterState = "DOWN"     // cluster is unable to serve queries.

etcd/embed.go

@@ -490,7 +490,6 @@ func (e *Etcd) ClusterState(ctx context.Context) (out disco.ClusterState, err er
 	}
 	var (
 		heartbeats int = 0
-		starting   bool
 	)
 	e.nodeMu.Lock()
 	nodes := e.populateNodeStates(ctx)
@@ -500,28 +499,16 @@ func (e *Etcd) ClusterState(ctx context.Context) (out disco.ClusterState, err er
 		return disco.ClusterStateUnknown, err
 	}
 	for _, node := range nodes {
-		switch node.State {
-		case disco.NodeStateStarting:
-			starting = true
-		case disco.NodeStateUnknown:
-			continue
+		if node.State == disco.NodeStateStarted {
+			heartbeats++
 		}
-
-		heartbeats++
 	}
-
-	if starting {
-		return disco.ClusterStateStarting, nil
-	}
-
 	if heartbeats < len(e.knownNodes) {
 		if len(e.knownNodes)-heartbeats >= e.replicas {
 			return disco.ClusterStateDown, nil
 		}
-
 		return disco.ClusterStateDegraded, nil
 	}
-
 	return disco.ClusterStateNormal, nil
 }
 

etcd/leasedkv_test.go

@@ -29,26 +29,23 @@ func TestClusterKv(t *testing.T) {
 	if err != nil {
 		t.Fatalf("starting cluster: %v", err)
 	}
-	c.MustAwaitClusterState(disco.ClusterStateStarting, 10*time.Second)
+	c.MustAwaitClusterState(disco.ClusterStateDown, 10*time.Second)
 	err = c.BringUp()
 	if err != nil {
 		t.Fatalf("bringing up cluster: %v", err)
 	}
 	c.MustAwaitClusterState(disco.ClusterStateNormal, 10*time.Second)
-	_, err = c.Elect()
-	if err != nil {
-		t.Fatalf("trying to cause election: %v", err)
-	}
 	ctx := context.TODO()
 	c.nodes[0].SetState(ctx, disco.NodeStateStarting)
 	c.nodes[1].SetState(ctx, disco.NodeStateStarting)
-	c.MustAwaitClusterState(disco.ClusterStateStarting, 10*time.Second)
+	c.MustAwaitClusterState(disco.ClusterStateDown, 10*time.Second)
 	c.nodes[0].SetState(ctx, disco.NodeStateStarted)
-	c.nodes[1].SetState(ctx, disco.NodeStateStarted)
 	// Two of three nodes are up, one is down, we have 2 replicas, so
 	// we should be able to handle reads but not writes, so we're in
 	// a Degraded state.
 	c.MustAwaitClusterState(disco.ClusterStateDegraded, 10*time.Second)
+	c.nodes[1].SetState(ctx, disco.NodeStateStarted)
+	c.MustAwaitClusterState(disco.ClusterStateNormal, 10*time.Second)
 	err = c.Stop()
 	if err != nil {
 		t.Fatalf("stopping cluster: %v", err)

server.go

@@ -694,29 +694,29 @@ func (s *Server) Open() error {
 		if !timer.Stop() {
 			<-timer.C
 		}
+		// wait for cluster to achieve Normal state
+		//
+		// This used to loop as long as the cluster was Starting, Down,
+		// or Unknown. It would come up in a Degraded state, except
+		// that during startup, as long as at least one node was Starting,
+		// we'd stay in Starting rather than Degraded. We've dropped the
+		// special case of Starting state, so now we just want to wait
+		// for Normal.
 		for {
 			state, err := s.noder.ClusterState(ctx)
 			if err != nil {
 				s.logger.Printf("failed to check cluster state: %v", err)
-				timer.Reset(time.Second)
-				select {
-				case <-s.closing:
-					return
-				case <-timer.C:
-					continue
-				}
 			}
-			switch state {
-			case disco.ClusterStateStarting, disco.ClusterStateUnknown, disco.ClusterStateDown:
-				timer.Reset(time.Second)
-				select {
-				case <-s.closing:
-					return
-				case <-timer.C:
-					continue
-				}
+			if state == disco.ClusterStateNormal {
+				break
+			}
+			timer.Reset(time.Second)
+			select {
+			case <-s.closing:
+				return
+			case <-timer.C:
+				continue
 			}
-			break
 		}
 
 		start := time.Now()