storeliveness: add LastSupportWithdrawnTime to InspectZ output

Following up on the previous commit, we make the
LastSupportWithdrawnTime observable via inspectz and the crdb_internal
schema. Most of this commit is splitting out the SupportFor and
SupportFrom inspectz codepaths, as they can no longer use the same
underlying slpb.SupportState.

Informs #158513

Release note: None

Author: arulajmani

pkg/inspectz/inspectz.go

@@ -59,11 +59,11 @@ func NewServer(
 	mux.Handle("/inspectz/v2/kvflowcontroller", server.makeKVFlowControllerHandler(server.KVFlowControllerV2))
 	mux.Handle(
 		"/inspectz/storeliveness/supportFrom",
-		server.makeStoreLivenessHandler(server.StoreLivenessSupportFrom),
+		server.makeStoreLivenessSupportFromHandler(server.StoreLivenessSupportFrom),
 	)
 	mux.Handle(
 		"/inspectz/storeliveness/supportFor",
-		server.makeStoreLivenessHandler(server.StoreLivenessSupportFor),
+		server.makeStoreLivenessSupportForHandler(server.StoreLivenessSupportFor),
 	)
 
 	return server
@@ -112,9 +112,27 @@ func (s *Server) makeKVFlowControllerHandler(
 	}
 }
 
-func (s *Server) makeStoreLivenessHandler(
+func (s *Server) makeStoreLivenessSupportFromHandler(
 	impl func(ctx context.Context, request *slpb.InspectStoreLivenessRequest) (
-		*slpb.InspectStoreLivenessResponse, error,
+		*slpb.InspectSupportFromResponse, error,
+	),
+) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		ctx := s.AnnotateCtx(context.Background())
+		req := &slpb.InspectStoreLivenessRequest{}
+		resp, err := impl(ctx, req)
+		if err != nil {
+			log.Dev.ErrorfDepth(ctx, 1, "%s", err)
+			http.Error(w, "internal error: check logs for details", http.StatusInternalServerError)
+			return
+		}
+		respond(ctx, w, http.StatusOK, resp)
+	}
+}
+
+func (s *Server) makeStoreLivenessSupportForHandler(
+	impl func(ctx context.Context, request *slpb.InspectStoreLivenessRequest) (
+		*slpb.InspectSupportForResponse, error,
 	),
 ) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
@@ -147,20 +165,20 @@ func (s *Server) KVFlowHandlesV2(
 // StoreLivenessSupportFrom implements the InspectzServer interface.
 func (s *Server) StoreLivenessSupportFrom(
 	_ context.Context, _ *slpb.InspectStoreLivenessRequest,
-) (*slpb.InspectStoreLivenessResponse, error) {
-	resp := &slpb.InspectStoreLivenessResponse{}
+) (*slpb.InspectSupportFromResponse, error) {
+	resp := &slpb.InspectSupportFromResponse{}
 	support, err := s.storeLiveness.InspectAllSupportFrom()
-	resp.SupportStatesPerStore = support
+	resp.SupportFromStatesPerStore = support
 	return resp, err
 }
 
 // StoreLivenessSupportFor implements the InspectzServer interface.
 func (s *Server) StoreLivenessSupportFor(
 	_ context.Context, _ *slpb.InspectStoreLivenessRequest,
-) (*slpb.InspectStoreLivenessResponse, error) {
-	resp := &slpb.InspectStoreLivenessResponse{}
+) (*slpb.InspectSupportForResponse, error) {
+	resp := &slpb.InspectSupportForResponse{}
 	support, err := s.storeLiveness.InspectAllSupportFor()
-	resp.SupportStatesPerStore = support
+	resp.SupportForStatesPerStore = support
 	return resp, err
 }
 

pkg/inspectz/inspectzpb/inspectz.proto

@@ -32,13 +32,13 @@ service Inspectz {
   // storeliveness.SupportManagers' views of support provided from other stores.
   // It's housed under /inspectz/storeliveness/supportFrom.
   rpc StoreLivenessSupportFrom(kv.kvserver.storeliveness.storelivenesspb.InspectStoreLivenessRequest)
-      returns (kv.kvserver.storeliveness.storelivenesspb.InspectStoreLivenessResponse) {}
+      returns (kv.kvserver.storeliveness.storelivenesspb.InspectSupportFromResponse) {}
 
   // StoreLivenessSupportFor exposes the in-memory state of all stores'
   // storeliveness.SupportManagers' views of support provided for other stores.
   // It's housed under /inspectz/storeliveness/supportFor.
   rpc StoreLivenessSupportFor(kv.kvserver.storeliveness.storelivenesspb.InspectStoreLivenessRequest)
-      returns (kv.kvserver.storeliveness.storelivenesspb.InspectStoreLivenessResponse) {}
+      returns (kv.kvserver.storeliveness.storelivenesspb.InspectSupportForResponse) {}
 
 }
 

pkg/inspectz/unsupported.go

@@ -51,13 +51,13 @@ func (u Unsupported) KVFlowHandlesV2(
 // StoreLivenessSupportFrom is part of the inspectzpb.InspectzServer interface.
 func (u Unsupported) StoreLivenessSupportFrom(
 	_ context.Context, _ *slpb.InspectStoreLivenessRequest,
-) (*slpb.InspectStoreLivenessResponse, error) {
+) (*slpb.InspectSupportFromResponse, error) {
 	return nil, errorutil.UnsupportedUnderClusterVirtualization(errorutil.FeatureNotAvailableToNonSystemTenantsIssue)
 }
 
 // StoreLivenessSupportFor is part of the inspectzpb.InspectzServer interface.
 func (u Unsupported) StoreLivenessSupportFor(
 	_ context.Context, _ *slpb.InspectStoreLivenessRequest,
-) (*slpb.InspectStoreLivenessResponse, error) {
+) (*slpb.InspectSupportForResponse, error) {
 	return nil, errorutil.UnsupportedUnderClusterVirtualization(errorutil.FeatureNotAvailableToNonSystemTenantsIssue)
 }

pkg/kv/kvserver/storeliveness/fabric.go

@@ -73,6 +73,6 @@ type Fabric interface {
 // InspectFabric is an interface that exposes all in-memory support state for a
 // given store. It is used to power the Store Liveness /inspectz functionality.
 type InspectFabric interface {
-	InspectSupportFrom() slpb.SupportStatesPerStore
-	InspectSupportFor() slpb.SupportStatesPerStore
+	InspectSupportFrom() slpb.InspectSupportFromStatesPerStore
+	InspectSupportFor() slpb.InspectSupportForStatesPerStore
 }

pkg/kv/kvserver/storeliveness/storelivenesspb/service.proto

@@ -155,19 +155,42 @@ message SupportState {
   util.hlc.Timestamp expiration = 3 [(gogoproto.nullable) = false];
 }
 
+// InspectSupportForState is used for observability via inspectz. It wraps
+// SupportState and includes additional non-persisted metadata like
+// LastSupportWithdrawnTime.
+message InspectSupportForState {
+  SupportState       support_state                = 1 [(gogoproto.nullable) = false, (gogoproto.embed) = true];
+  // LastSupportWithdrawnTime is the timestamp at which support was last
+  // withdrawn from the remote store. This field is not persisted to disk.
+  util.hlc.Timestamp last_support_withdrawn_time  = 2 [(gogoproto.nullable) = false];
+}
+
 // InspectStoreLivenessRequest is used to power the Store Liveness /inspectz
 // functionality. The request doesn't take any parameters.
 message InspectStoreLivenessRequest {}
 
-// InspectStoreLivenessRequest is used to power the Store Liveness /inspectz
-// functionality. The response is a list of SupportStatesPerStore.
-message InspectStoreLivenessResponse {
-  repeated SupportStatesPerStore support_states_per_store = 1 [(gogoproto.nullable) = false];
+// InspectSupportFromResponse is used to power the Store Liveness /inspectz
+// supportFrom functionality.
+message InspectSupportFromResponse {
+  repeated InspectSupportFromStatesPerStore support_from_states_per_store = 1 [(gogoproto.nullable) = false];
+}
+
+// InspectSupportForResponse is used to power the Store Liveness /inspectz
+// supportFor functionality.
+message InspectSupportForResponse {
+  repeated InspectSupportForStatesPerStore support_for_states_per_store = 1 [(gogoproto.nullable) = false];
+}
+
+// InspectSupportFromStatesPerStore includes all SupportStates for a given
+// store; they correspond to the support-from map of a given store.
+message InspectSupportFromStatesPerStore {
+  StoreIdent store_id = 1 [(gogoproto.nullable) = false, (gogoproto.customname) = "StoreID"];
+  repeated SupportState support_from_states = 2 [(gogoproto.nullable) = false];
 }
 
-// SupportStatesPerStore includes all SupportStates for a given store; they
-// correspond to either the support-from or support-for map of a given store.
-message SupportStatesPerStore {
+// InspectSupportForStatesPerStore includes all InspectSupportForStates for a
+// given store; they correspond to the support-for map of a given store.
+message InspectSupportForStatesPerStore {
   StoreIdent store_id = 1 [(gogoproto.nullable) = false, (gogoproto.customname) = "StoreID"];
-  repeated SupportState support_states = 2 [(gogoproto.nullable) = false];
+  repeated InspectSupportForState support_for_states = 2 [(gogoproto.nullable) = false];
 }

pkg/kv/kvserver/storeliveness/support_manager.go

@@ -121,15 +121,15 @@ func (sm *SupportManager) SupportFor(id slpb.StoreIdent) (slpb.Epoch, bool) {
 }
 
 // InspectSupportFrom implements the InspectFabric interface.
-func (sm *SupportManager) InspectSupportFrom() slpb.SupportStatesPerStore {
-	supportStates := sm.requesterStateHandler.exportAllSupportFrom()
-	return slpb.SupportStatesPerStore{StoreID: sm.storeID, SupportStates: supportStates}
+func (sm *SupportManager) InspectSupportFrom() slpb.InspectSupportFromStatesPerStore {
+	supportFromStates := sm.requesterStateHandler.exportAllSupportFrom()
+	return slpb.InspectSupportFromStatesPerStore{StoreID: sm.storeID, SupportFromStates: supportFromStates}
 }
 
 // InspectSupportFor implements the InspectFabric interface.
-func (sm *SupportManager) InspectSupportFor() slpb.SupportStatesPerStore {
-	supportStates := sm.supporterStateHandler.exportAllSupportFor()
-	return slpb.SupportStatesPerStore{StoreID: sm.storeID, SupportStates: supportStates}
+func (sm *SupportManager) InspectSupportFor() slpb.InspectSupportForStatesPerStore {
+	supportForStates := sm.supporterStateHandler.exportAllSupportFor()
+	return slpb.InspectSupportForStatesPerStore{StoreID: sm.storeID, SupportForStates: supportForStates}
 }
 
 // SupportFrom implements the Fabric interface. It delegates the response to the

pkg/kv/kvserver/storeliveness/supporter_state.go

@@ -122,16 +122,19 @@ func (ssh *supporterStateHandler) getNumSupportFor() int {
 	return len(ssh.supporterState.supportFor)
 }
 
-// exportAllSupportFor exports a copy of all SupportStates from the
+// exportAllSupportFor exports a copy of all InspectSupportForStates from the
 // supporterState.supportFor map.
-func (ssh *supporterStateHandler) exportAllSupportFor() []slpb.SupportState {
+func (ssh *supporterStateHandler) exportAllSupportFor() []slpb.InspectSupportForState {
 	ssh.mu.RLock()
 	defer ssh.mu.RUnlock()
-	supportStates := make([]slpb.SupportState, 0, len(ssh.supporterState.supportFor))
+	supportForStates := make([]slpb.InspectSupportForState, 0, len(ssh.supporterState.supportFor))
 	for _, ss := range ssh.supporterState.supportFor {
-		supportStates = append(supportStates, ss.SupportState)
+		supportForStates = append(supportForStates, slpb.InspectSupportForState{
+			SupportState:             ss.SupportState,
+			LastSupportWithdrawnTime: ss.lastSupportWithdrawnTime,
+		})
 	}
-	return supportStates
+	return supportForStates
 }
 
 // Functions for handling supporterState updates.

pkg/kv/kvserver/stores_store_liveness.go

@@ -11,8 +11,8 @@ import slpb "github.com/cockroachdb/cockroach/pkg/kv/kvserver/storeliveness/stor
 // Liveness state to be combined into a per-node view. It powers the inspectz
 // Store Liveness functionality.
 type InspectAllStoreLiveness interface {
-	InspectAllSupportFrom() ([]slpb.SupportStatesPerStore, error)
-	InspectAllSupportFor() ([]slpb.SupportStatesPerStore, error)
+	InspectAllSupportFrom() ([]slpb.InspectSupportFromStatesPerStore, error)
+	InspectAllSupportFor() ([]slpb.InspectSupportForStatesPerStore, error)
 }
 
 // StoresForStoreLiveness is a wrapper around Stores that implements
@@ -27,29 +27,35 @@ func MakeStoresForStoreLiveness(stores *Stores) *StoresForStoreLiveness {
 }
 
 // InspectAllSupportFrom implements the InspectAllStoreLiveness interface. It
-// iterates over all stores and aggregates their SupportFrom SupportStates.
-func (sfsl *StoresForStoreLiveness) InspectAllSupportFrom() ([]slpb.SupportStatesPerStore, error) {
+// iterates over all stores and aggregates their SupportFrom states.
+func (sfsl *StoresForStoreLiveness) InspectAllSupportFrom() (
+	[]slpb.InspectSupportFromStatesPerStore,
+	error,
+) {
 	stores := (*Stores)(sfsl)
-	var sspf []slpb.SupportStatesPerStore
+	var sfsps []slpb.InspectSupportFromStatesPerStore
 	err := stores.VisitStores(
 		func(s *Store) error {
-			sspf = append(sspf, s.storeLiveness.InspectSupportFrom())
+			sfsps = append(sfsps, s.storeLiveness.InspectSupportFrom())
 			return nil
 		},
 	)
-	return sspf, err
+	return sfsps, err
 }
 
 // InspectAllSupportFor implements the InspectAllStoreLiveness interface. It
-// iterates over all stores and aggregates their SupportFor SupportStates.
-func (sfsl *StoresForStoreLiveness) InspectAllSupportFor() ([]slpb.SupportStatesPerStore, error) {
+// iterates over all stores and aggregates their SupportFor states.
+func (sfsl *StoresForStoreLiveness) InspectAllSupportFor() (
+	[]slpb.InspectSupportForStatesPerStore,
+	error,
+) {
 	stores := (*Stores)(sfsl)
-	var sspf []slpb.SupportStatesPerStore
+	var sfsps []slpb.InspectSupportForStatesPerStore
 	err := stores.VisitStores(
 		func(s *Store) error {
-			sspf = append(sspf, s.storeLiveness.InspectSupportFor())
+			sfsps = append(sfsps, s.storeLiveness.InspectSupportFor())
 			return nil
 		},
 	)
-	return sspf, err
+	return sfsps, err
 }

pkg/sql/crdb_internal.go

@@ -9627,20 +9627,21 @@ CREATE TABLE crdb_internal.store_liveness_support_from (
 		if err != nil {
 			return err
 		}
-		return populateStoreLivenessSupportResponse(resp, addRow)
+		return populateStoreLivenessSupportFromResponse(resp, addRow)
 	},
 }
 
 var crdbInternalStoreLivenessSupportForTable = virtualSchemaTable{
 	comment: `node-level view of store liveness support for other stores`,
 	schema: `
 CREATE TABLE crdb_internal.store_liveness_support_for (
-  node_id               INT NOT NULL,
-  store_id              INT NOT NULL,
-  support_for_node_id  INT NOT NULL,
-  support_for_store_id INT NOT NULL,
-  support_epoch         INT NOT NULL,
-  support_expiration    TIMESTAMP NOT NULL
+  node_id                        INT NOT NULL,
+  store_id                       INT NOT NULL,
+  support_for_node_id            INT NOT NULL,
+  support_for_store_id           INT NOT NULL,
+  support_epoch                  INT NOT NULL,
+  support_expiration             TIMESTAMP NOT NULL,
+  last_support_withdrawn_time    TIMESTAMP NOT NULL
 );`,
 	populate: func(ctx context.Context, p *planner, _ catalog.DatabaseDescriptor, addRow func(...tree.Datum) error) error {
 		hasRoleOption, _, err := p.HasViewActivityOrViewActivityRedactedRole(ctx)
@@ -9655,22 +9656,43 @@ CREATE TABLE crdb_internal.store_liveness_support_for (
 		if err != nil {
 			return err
 		}
-		return populateStoreLivenessSupportResponse(resp, addRow)
+		return populateStoreLivenessSupportForResponse(resp, addRow)
 	},
 }
 
-func populateStoreLivenessSupportResponse(
-	resp *slpb.InspectStoreLivenessResponse, addRow func(...tree.Datum) error,
+func populateStoreLivenessSupportFromResponse(
+	resp *slpb.InspectSupportFromResponse, addRow func(...tree.Datum) error,
+) error {
+	for _, sfsps := range resp.SupportFromStatesPerStore {
+		for _, sfs := range sfsps.SupportFromStates {
+			if err := addRow(
+				tree.NewDInt(tree.DInt(sfsps.StoreID.NodeID)),
+				tree.NewDInt(tree.DInt(sfsps.StoreID.StoreID)),
+				tree.NewDInt(tree.DInt(sfs.Target.NodeID)),
+				tree.NewDInt(tree.DInt(sfs.Target.StoreID)),
+				tree.NewDInt(tree.DInt(sfs.Epoch)),
+				eval.TimestampToInexactDTimestamp(sfs.Expiration),
+			); err != nil {
+				return err
+			}
+		}
+	}
+	return nil
+}
+
+func populateStoreLivenessSupportForResponse(
+	resp *slpb.InspectSupportForResponse, addRow func(...tree.Datum) error,
 ) error {
-	for _, ssps := range resp.SupportStatesPerStore {
-		for _, ss := range ssps.SupportStates {
+	for _, sfsps := range resp.SupportForStatesPerStore {
+		for _, sfs := range sfsps.SupportForStates {
 			if err := addRow(
-				tree.NewDInt(tree.DInt(ssps.StoreID.NodeID)),
-				tree.NewDInt(tree.DInt(ssps.StoreID.StoreID)),
-				tree.NewDInt(tree.DInt(ss.Target.NodeID)),
-				tree.NewDInt(tree.DInt(ss.Target.StoreID)),
-				tree.NewDInt(tree.DInt(ss.Epoch)),
-				eval.TimestampToInexactDTimestamp(ss.Expiration),
+				tree.NewDInt(tree.DInt(sfsps.StoreID.NodeID)),
+				tree.NewDInt(tree.DInt(sfsps.StoreID.StoreID)),
+				tree.NewDInt(tree.DInt(sfs.Target.NodeID)),
+				tree.NewDInt(tree.DInt(sfs.Target.StoreID)),
+				tree.NewDInt(tree.DInt(sfs.Epoch)),
+				eval.TimestampToInexactDTimestamp(sfs.Expiration),
+				eval.TimestampToInexactDTimestamp(sfs.LastSupportWithdrawnTime),
 			); err != nil {
 				return err
 			}