Merge #154573 #154706

154573: sql/inspect: filter SHOW INSPECT ERRORS by job payload r=spilchen a=spilchen

Previously, `SHOW INSPECT ERRORS FOR TABLE` would select the most recent completed INSPECT job that had errors for that table by joining with the `inspect_errors` table. This approach had a flaw: if a more recent job inspected the table but found no errors, it would be ignored in favor of an older job with errors.

This changes that by query the INSPECT job payload to determine which jobs actually inspected a given table.

Informs #148287

Release note: none
Epic: CRDB-30356

154706: kvcoord: remove metrics for deprecated dist sender errors r=yuzefovich a=yuzefovich

Epic: None
Release note: None

Co-authored-by: Matt Spilchen <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>

Author: 3 people

docs/generated/metrics/metrics.yaml

@@ -2743,146 +2743,6 @@ layers:
       unit: COUNT
       aggregation: AVG
       derivative: NON_NEGATIVE_DERIVATIVE
-    - name: distsender.rpc.err.errordetailtype(0)
-      exported_name: distsender_rpc_err_errordetailtype_0_
-      description: |
-        Number of ErrorDetailType(0) errors received replica-bound RPCs
-
-        This counts how often error of the specified type was received back from replicas
-        as part of executing possibly range-spanning requests. Failures to reach the target
-        replica will be accounted for as 'roachpb.CommunicationErrType' and unclassified
-        errors as 'roachpb.InternalErrType'.
-      y_axis_label: Errors
-      type: COUNTER
-      unit: COUNT
-      aggregation: AVG
-      derivative: NON_NEGATIVE_DERIVATIVE
-    - name: distsender.rpc.err.errordetailtype(15)
-      exported_name: distsender_rpc_err_errordetailtype_15_
-      description: |
-        Number of ErrorDetailType(15) errors received replica-bound RPCs
-
-        This counts how often error of the specified type was received back from replicas
-        as part of executing possibly range-spanning requests. Failures to reach the target
-        replica will be accounted for as 'roachpb.CommunicationErrType' and unclassified
-        errors as 'roachpb.InternalErrType'.
-      y_axis_label: Errors
-      type: COUNTER
-      unit: COUNT
-      aggregation: AVG
-      derivative: NON_NEGATIVE_DERIVATIVE
-    - name: distsender.rpc.err.errordetailtype(19)
-      exported_name: distsender_rpc_err_errordetailtype_19_
-      description: |
-        Number of ErrorDetailType(19) errors received replica-bound RPCs
-
-        This counts how often error of the specified type was received back from replicas
-        as part of executing possibly range-spanning requests. Failures to reach the target
-        replica will be accounted for as 'roachpb.CommunicationErrType' and unclassified
-        errors as 'roachpb.InternalErrType'.
-      y_axis_label: Errors
-      type: COUNTER
-      unit: COUNT
-      aggregation: AVG
-      derivative: NON_NEGATIVE_DERIVATIVE
-    - name: distsender.rpc.err.errordetailtype(20)
-      exported_name: distsender_rpc_err_errordetailtype_20_
-      description: |
-        Number of ErrorDetailType(20) errors received replica-bound RPCs
-
-        This counts how often error of the specified type was received back from replicas
-        as part of executing possibly range-spanning requests. Failures to reach the target
-        replica will be accounted for as 'roachpb.CommunicationErrType' and unclassified
-        errors as 'roachpb.InternalErrType'.
-      y_axis_label: Errors
-      type: COUNTER
-      unit: COUNT
-      aggregation: AVG
-      derivative: NON_NEGATIVE_DERIVATIVE
-    - name: distsender.rpc.err.errordetailtype(21)
-      exported_name: distsender_rpc_err_errordetailtype_21_
-      description: |
-        Number of ErrorDetailType(21) errors received replica-bound RPCs
-
-        This counts how often error of the specified type was received back from replicas
-        as part of executing possibly range-spanning requests. Failures to reach the target
-        replica will be accounted for as 'roachpb.CommunicationErrType' and unclassified
-        errors as 'roachpb.InternalErrType'.
-      y_axis_label: Errors
-      type: COUNTER
-      unit: COUNT
-      aggregation: AVG
-      derivative: NON_NEGATIVE_DERIVATIVE
-    - name: distsender.rpc.err.errordetailtype(23)
-      exported_name: distsender_rpc_err_errordetailtype_23_
-      description: |
-        Number of ErrorDetailType(23) errors received replica-bound RPCs
-
-        This counts how often error of the specified type was received back from replicas
-        as part of executing possibly range-spanning requests. Failures to reach the target
-        replica will be accounted for as 'roachpb.CommunicationErrType' and unclassified
-        errors as 'roachpb.InternalErrType'.
-      y_axis_label: Errors
-      type: COUNTER
-      unit: COUNT
-      aggregation: AVG
-      derivative: NON_NEGATIVE_DERIVATIVE
-    - name: distsender.rpc.err.errordetailtype(24)
-      exported_name: distsender_rpc_err_errordetailtype_24_
-      description: |
-        Number of ErrorDetailType(24) errors received replica-bound RPCs
-
-        This counts how often error of the specified type was received back from replicas
-        as part of executing possibly range-spanning requests. Failures to reach the target
-        replica will be accounted for as 'roachpb.CommunicationErrType' and unclassified
-        errors as 'roachpb.InternalErrType'.
-      y_axis_label: Errors
-      type: COUNTER
-      unit: COUNT
-      aggregation: AVG
-      derivative: NON_NEGATIVE_DERIVATIVE
-    - name: distsender.rpc.err.errordetailtype(29)
-      exported_name: distsender_rpc_err_errordetailtype_29_
-      description: |
-        Number of ErrorDetailType(29) errors received replica-bound RPCs
-
-        This counts how often error of the specified type was received back from replicas
-        as part of executing possibly range-spanning requests. Failures to reach the target
-        replica will be accounted for as 'roachpb.CommunicationErrType' and unclassified
-        errors as 'roachpb.InternalErrType'.
-      y_axis_label: Errors
-      type: COUNTER
-      unit: COUNT
-      aggregation: AVG
-      derivative: NON_NEGATIVE_DERIVATIVE
-    - name: distsender.rpc.err.errordetailtype(30)
-      exported_name: distsender_rpc_err_errordetailtype_30_
-      description: |
-        Number of ErrorDetailType(30) errors received replica-bound RPCs
-
-        This counts how often error of the specified type was received back from replicas
-        as part of executing possibly range-spanning requests. Failures to reach the target
-        replica will be accounted for as 'roachpb.CommunicationErrType' and unclassified
-        errors as 'roachpb.InternalErrType'.
-      y_axis_label: Errors
-      type: COUNTER
-      unit: COUNT
-      aggregation: AVG
-      derivative: NON_NEGATIVE_DERIVATIVE
-    - name: distsender.rpc.err.errordetailtype(33)
-      exported_name: distsender_rpc_err_errordetailtype_33_
-      description: |
-        Number of ErrorDetailType(33) errors received replica-bound RPCs
-
-        This counts how often error of the specified type was received back from replicas
-        as part of executing possibly range-spanning requests. Failures to reach the target
-        replica will be accounted for as 'roachpb.CommunicationErrType' and unclassified
-        errors as 'roachpb.InternalErrType'.
-      y_axis_label: Errors
-      type: COUNTER
-      unit: COUNT
-      aggregation: AVG
-      derivative: NON_NEGATIVE_DERIVATIVE
     - name: distsender.rpc.err.exclusionviolationerrtype
       exported_name: distsender_rpc_err_exclusionviolationerrtype
       description: |

pkg/kv/kvclient/kvcoord/dist_sender.go

@@ -460,8 +460,9 @@ type DistSenderMetrics struct {
 	ProxyForwardSentCount              *metric.Counter
 	ProxyForwardErrCount               *metric.Counter
 	MethodCounts                       [kvpb.NumMethods]*metric.Counter
-	ErrCounts                          [kvpb.NumErrors]*metric.Counter
-	CircuitBreaker                     DistSenderCircuitBreakerMetrics
+	// ErrCounts[i] can be nil if i'th error has been deprecated.
+	ErrCounts      [kvpb.NumErrors]*metric.Counter
+	CircuitBreaker DistSenderCircuitBreakerMetrics
 	DistSenderRangeFeedMetrics
 }
 
@@ -526,6 +527,10 @@ func MakeDistSenderMetrics(locality roachpb.Locality) DistSenderMetrics {
 	}
 	for i := range m.ErrCounts {
 		errType := kvpb.ErrorDetailType(i).String()
+		if strings.HasPrefix(errType, "ErrorDetailType") {
+			// This error index has been deprecated.
+			continue
+		}
 		meta := metaDistSenderErrCountTmpl
 		meta.Name = fmt.Sprintf(meta.Name, strings.ToLower(errType))
 		meta.Help = fmt.Sprintf(meta.Help, errType)
@@ -3187,14 +3192,18 @@ func (ds *DistSender) maybeIncrementErrCounters(br *kvpb.BatchResponse, err erro
 	if err == nil && br.Error == nil {
 		return
 	}
+	var counter *metric.Counter
 	if err != nil {
-		ds.metrics.ErrCounts[kvpb.CommunicationErrType].Inc(1)
+		counter = ds.metrics.ErrCounts[kvpb.CommunicationErrType]
 	} else {
 		typ := kvpb.InternalErrType
 		if detail := br.Error.GetDetail(); detail != nil {
 			typ = detail.Type()
 		}
-		ds.metrics.ErrCounts[typ].Inc(1)
+		counter = ds.metrics.ErrCounts[typ]
+	}
+	if counter != nil {
+		counter.Inc(1)
 	}
 }
 

pkg/sql/delegate/show_inspect_errors.go

@@ -57,25 +57,37 @@ func (d *delegator) delegateShowInspectErrors(n *tree.ShowInspectErrors) (tree.S
 		query.WriteString(fmt.Sprintf(" AND id = %d", *n.JobID))
 	}
 
-	// TODO(148287): query the inspect job payload to figure out if a job touches a particular table or database ID
-	// If a table was specified, only consider jobs that reported errors on it.
-	// If a job ID was specified, only consider that job. The records from the
-	// most recent completed job that satisfies those criteria is used.
 	query.WriteString(`),
-	job_id AS (
-		SELECT max(inspect_jobs.id) as id
+	filtered_jobs AS (
+		SELECT inspect_jobs.id
 		FROM inspect_jobs
-		JOIN crdb_internal.cluster_inspect_errors ie ON inspect_jobs.id = ie.job_id
-		WHERE 1=1
+		JOIN crdb_internal.system_jobs sj ON inspect_jobs.id = sj.id
 	`)
+	// If a table is specified, limit results to jobs that include checks
+	// on that table. The table is matched by inspecting the job payload.
 	if tableID != catid.InvalidDescID {
-		query.WriteString(fmt.Sprintf(" AND ie.id = %d", tableID))
-	}
-	if n.JobID != nil {
-		query.WriteString(fmt.Sprintf(" AND ie.job_id = %d", *n.JobID))
+		query.WriteString(fmt.Sprintf(`
+		WHERE EXISTS (
+			SELECT 1
+			FROM jsonb_array_elements(
+				COALESCE(
+					crdb_internal.pb_to_json('cockroach.sql.jobs.jobspb.Payload', sj.payload)
+						-> 'inspectDetails' -> 'checks',
+					'[]'::JSONB
+				)
+			) AS c
+			WHERE (c ->> 'tableId')::INT = %d
+		)`, tableID))
 	}
+	// Reports on a single job. If multiple match, use the most recent one.
+	query.WriteString(`),
+	job_id AS (
+		SELECT max(id) AS id
+		FROM filtered_jobs
+	)
+	`)
 
-	query.WriteString(`)
+	query.WriteString(`
 	SELECT 
 		ie.error_type,
 		COALESCE(t.database_name, '<unknown>') AS database_name,