Merge #142930

142930: server: new endpoint to expose node criticality status r=[arulajmani,kyle-a-wong] a=MattWhelan

We expose range status via the cluster API metrics, which is sufficient to derive node criticality information. However, it's complicated, and customers want a simple, opinionated way to know whether a particular node is safe to terminate when orchestrating a rolling restart.

This change exposes a new endpoint, analogous to a healthcheck, but instead of indicating node health, it indicates when it's unsafe to terminate a node due to impact on cluster quorum health.

Epic: none
Part of: https://cockroachlabs.atlassian.net/browse/TREQ-929

Release note (ops change): The restart safety endpoint indicates when it is unsafe to terminate a node.

Co-authored-by: Matt Whelan <[email protected]>

Author: craig[bot]

pkg/server/BUILD.bazel

@@ -162,6 +162,7 @@ go_library(
         "//pkg/multitenant/tenantcapabilitiespb",
         "//pkg/multitenant/tenantcostmodel",
         "//pkg/raft",
+        "//pkg/raft/raftpb",
         "//pkg/roachpb",
         "//pkg/rpc",
         "//pkg/rpc/nodedialer",

pkg/server/api_v2.go

@@ -33,6 +33,11 @@ import (
 	"strconv"
 
 	"github.com/cockroachdb/cockroach/pkg/kv"
+	"github.com/cockroachdb/cockroach/pkg/kv/kvserver"
+	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/allocator/allocatorimpl"
+	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness/livenesspb"
+	"github.com/cockroachdb/cockroach/pkg/raft/raftpb"
+	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/security/username"
 	"github.com/cockroachdb/cockroach/pkg/server/apiconstants"
 	"github.com/cockroachdb/cockroach/pkg/server/apiutil"
@@ -55,6 +60,7 @@ const (
 
 type ApiV2System interface {
 	health(w http.ResponseWriter, r *http.Request)
+	restartSafetyCheck(w http.ResponseWriter, r *http.Request)
 	listNodes(w http.ResponseWriter, r *http.Request)
 	listNodeRanges(w http.ResponseWriter, r *http.Request)
 }
@@ -95,7 +101,7 @@ type apiV2SystemServer struct {
 }
 
 var _ ApiV2System = &apiV2SystemServer{}
-var _ http.Handler = &apiV2Server{}
+var _ http.Handler = &apiV2SystemServer{}
 
 // newAPIV2Server returns a new apiV2Server.
 func newAPIV2Server(ctx context.Context, opts *apiV2ServerOpts) http.Handler {
@@ -180,6 +186,7 @@ func registerRoutes(
 		{"nodes/{node_id}/ranges/", systemRoutes.listNodeRanges, true, authserver.ViewClusterMetadataRole, false},
 		{"ranges/hot/", a.listHotRanges, true, authserver.ViewClusterMetadataRole, false},
 		{"ranges/{range_id:[0-9]+}/", a.listRange, true, authserver.ViewClusterMetadataRole, false},
+		{"health/restart_safety/", systemRoutes.restartSafetyCheck, false, authserver.RegularRole, false},
 		{"health/", systemRoutes.health, false, authserver.RegularRole, false},
 		{"users/", a.listUsers, true, authserver.RegularRole, false},
 		{"events/", a.listEvents, true, authserver.ViewClusterMetadataRole, false},
@@ -394,6 +401,169 @@ func (a *apiV2Server) health(w http.ResponseWriter, r *http.Request) {
 	healthInternal(w, r, a.admin.checkReadinessForHealthCheck)
 }
 
+func (a *apiV2Server) restartSafetyCheck(w http.ResponseWriter, r *http.Request) {
+	apiutil.WriteJSONResponse(r.Context(), w, http.StatusNotImplemented, nil)
+}
+
+// # Restart Safety
+//
+// Endpoint to expose restart safety status. A 200 response indicates that
+// terminating the node in question won't cause any ranges to become
+// unavailable, at the time the response was prepared. Users may use this check
+// as a precondition for advancing a rolling restart process. Checks fail with
+// a 503, or a 500 in the case of an error evaluating safety.
+func (a *apiV2SystemServer) restartSafetyCheck(w http.ResponseWriter, r *http.Request) {
+	ctx := r.Context()
+
+	if r.Method != http.MethodGet {
+		http.Error(w, http.StatusText(http.StatusMethodNotAllowed), http.StatusMethodNotAllowed)
+		return
+	}
+
+	const AllowMinimumQuorumFlag = "allow_minimum_quorum"
+
+	query := r.URL.Query()
+	allowMinimumQuorum := false
+	var err error
+	if query.Has(AllowMinimumQuorumFlag) {
+		allowMinimumQuorum, err = strconv.ParseBool(query.Get(AllowMinimumQuorumFlag))
+		if err != nil {
+			http.Error(w, "invalid allow_minimum_quorum value; should be true or false", http.StatusBadRequest)
+			return
+		}
+	}
+
+	nodeID := a.systemStatus.node.Descriptor.NodeID
+
+	res, err := checkRestartSafe(
+		ctx,
+		nodeID,
+		a.systemStatus.nodeLiveness,
+		a.systemStatus.stores,
+		a.systemStatus.storePool.ClusterNodeCount(),
+		allowMinimumQuorum,
+	)
+	if err != nil {
+		http.Error(w, "Error checking store status", http.StatusInternalServerError)
+		return
+	}
+
+	if !res.IsRestartSafe {
+		// In the style of health check endpoints, we respond with a server error
+		// to indicate unhealthy. This makes it much easier to integrate this
+		// endpoint with relatively unsophisticated clients, such as shell scripts.
+		apiutil.WriteJSONResponse(ctx, w, http.StatusServiceUnavailable, res)
+		return
+	}
+	apiutil.WriteJSONResponse(ctx, w, http.StatusOK, res)
+}
+
+type storeVisitor interface {
+	VisitStores(visitor func(s *kvserver.Store) error) error
+}
+
+// RestartSafetyResponse indicates whether the current node is critical
+// (cannot be restarted safely).
+type RestartSafetyResponse struct {
+	NodeID int32 `json:"node_id,omitempty"`
+	// IsRestartSafe is true if restarting this node is safe, under the
+	// quorum restrictions requested
+	IsRestartSafe bool `json:"is_restart_safe,omitempty"`
+	// UnavailableRangeCount indicates how many currently unavailable
+	// ranges contribute to restart being unsafe.
+	UnavailableRangeCount int32 `json:"unavailable_range_count,omitempty"`
+	// UnderreplicatedRangeCount indicates how many currently
+	// underreplicated ranges contribute to restart being unsafe.
+	UnderreplicatedRangeCount int32 `json:"underreplicated_range_count,omitempty"`
+	// RaftLeadershipOnNodeCount indicates how many ranges this node leads.
+	RaftLeadershipOnNodeCount int32 `json:"raft_leadership_on_node_count,omitempty"`
+	// StoreNotDrainingCount indicates how many of this node's stores are
+	// not draining.
+	StoreNotDrainingCount int32 `json:"store_not_draining_count,omitempty"`
+}
+
+func checkRestartSafe(
+	ctx context.Context,
+	nodeID roachpb.NodeID,
+	nodeLiveness livenesspb.NodeVitalityInterface,
+	stores storeVisitor,
+	nodeCount int,
+	allowMinimumQuorum bool,
+) (*RestartSafetyResponse, error) {
+	res := &RestartSafetyResponse{
+		IsRestartSafe: true,
+		NodeID:        int32(nodeID),
+	}
+
+	vitality := nodeLiveness.ScanNodeVitalityFromCache()
+	// For each of the node's stores, check each replica's status.
+	err := stores.VisitStores(func(store *kvserver.Store) error {
+		if int32(store.NodeID()) != res.NodeID {
+			return nil
+		}
+
+		if !store.IsDraining() {
+			res.IsRestartSafe = false
+			res.StoreNotDrainingCount++
+		}
+
+		store.VisitReplicas(func(replica *kvserver.Replica) bool {
+			desc, spanCfg := replica.DescAndSpanConfig()
+
+			neededVoters := allocatorimpl.GetNeededVoters(spanCfg.GetNumVoters(), nodeCount)
+			rangeStatus := desc.Replicas().ReplicationStatus(func(rd roachpb.ReplicaDescriptor) bool {
+				return vitality[rd.NodeID].IsLive(livenesspb.Metrics)
+			}, neededVoters, -1)
+
+			isLeader := replica.RaftBasicStatus().RaftState == raftpb.StateLeader
+
+			// Reject Unavailable, Underreplicated, and Leader replicas as unsafe
+			// to terminate. Additionally, reject when the store (really the node,
+			// but the status is reported at the store level) is not draining.
+			if !rangeStatus.Available {
+				res.IsRestartSafe = false
+				res.UnavailableRangeCount++
+			}
+			if isLeader {
+				res.IsRestartSafe = false
+				res.RaftLeadershipOnNodeCount++
+			}
+
+			if rangeStatus.UnderReplicated {
+				if neededVoters >= 5 && allowMinimumQuorum {
+					// When neededVoters >= 5, present underreplication doesn't actually imply unavailability after we terminate
+					// this node. The caller has opted in to allowMinimumQuorum restarts, so check whether this node being down
+					// actually causes unavailability.
+					futureStatus := desc.Replicas().ReplicationStatus(func(rd roachpb.ReplicaDescriptor) bool {
+						if rd.NodeID == nodeID {
+							return false
+						}
+						return vitality[rd.NodeID].IsLive(livenesspb.Metrics)
+					}, neededVoters, -1)
+
+					if !futureStatus.Available {
+						// Even minimum quorum won't be maintained if we down this node.
+						res.IsRestartSafe = false
+						res.UnderreplicatedRangeCount++
+					}
+				} else {
+					// No allowMiniumQuorum (or not a big enough RF for that to matter), so under replication is enough.
+					res.IsRestartSafe = false
+					res.UnderreplicatedRangeCount++
+				}
+			}
+
+			return ctx.Err() == nil
+		})
+
+		if ctx.Err() != nil {
+			return ctx.Err()
+		}
+		return nil
+	})
+	return res, err
+}
+
 // # Get metric recording and alerting rule templates
 //
 // Endpoint to export recommended metric recording and alerting rules.