From 4467c18baabc6b64e9fd9cb1c97ae371c9a7903b Mon Sep 17 00:00:00 2001 From: Seebs Date: Mon, 31 Oct 2022 12:57:21 -0500 Subject: [PATCH] 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. (cherry picked from commit 52d3329434fb53060ccfd7e82f9bda870f67854f) --- api.go | 3 +-- disco/disco.go | 1 - etcd/embed.go | 17 ++--------------- etcd/leasedkv_test.go | 11 ++++------- server.go | 34 +++++++++++++++++----------------- 5 files changed, 24 insertions(+), 42 deletions(-) diff --git a/api.go b/api.go index 574b28344..2e9e85fec 100644 --- a/api.go +++ b/api.go @@ -119,8 +119,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 diff --git a/disco/disco.go b/disco/disco.go index 88933f002..ccc164705 100644 --- a/disco/disco.go +++ b/disco/disco.go @@ -55,7 +55,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. diff --git a/etcd/embed.go b/etcd/embed.go index 11162fd3b..7e05cd938 100644 --- a/etcd/embed.go +++ b/etcd/embed.go @@ -491,7 +491,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) @@ -501,28 +500,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 } diff --git a/etcd/leasedkv_test.go b/etcd/leasedkv_test.go index 8204f9fb1..2b9cdbb3e 100644 --- a/etcd/leasedkv_test.go +++ b/etcd/leasedkv_test.go @@ -30,26 +30,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) diff --git a/server.go b/server.go index 3fc24c077..536c44fa5 100644 --- a/server.go +++ b/server.go @@ -695,29 +695,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()