Merge #142826 #143598 #144091 #144188

142826: sql: add latency metrics for historical queries r=BramGruneir a=BramGruneir

This commit adds 4 new latency metrics specifically to separate out AOST queries:

* sql.exec.latency.consistent -- Latency of SQL statement execution of non-historical queries
* sql.exec.latency.historical -- Latency of SQL statement execution of historical queries
* sql.service.latency.consistent -- Latency of SQL request execution of non-historical queries
* sql.service.latency.historical -- Latency of SQL request execution of historical queries

This will help when trying to optimize workloads that have a combination of historical and non-historical queries.

Fixes: #121507
Part of: https://cockroachlabs.atlassian.net/browse/CRDB-37293
Part of: https://cockroachlabs.atlassian.net/browse/TREQ-152
Part of: https://cockroachlabs.atlassian.net/browse/FEB-22

Release note (ops change): Added 4 new latency metrics: sql.service.latency.historical, sql.service.latency.consistent, sql.exec.latency.historical, sql.exec.latency.consistent for better query
optimizations.

143598: roachtest: handle flaky node-postgres tests r=Dedej-Bergin a=Dedej-Bergin

Previously, we would only handle the "pool size of 1" flaky test in the node-postgres roachtest. However, there is another flaky test "events" which fails with "expected 0 to equal 20". This change updates the error handling to properly handle both known flaky tests while maintaining the existing behavior of failing on any other errors.

Fixes: #143047
Release note: none

144091: server: add information filtering to hot ranges endpoint r=angles-n-daemons a=angles-n-daemons

server: add information filtering to hot ranges endpoint

This change introduces two enhancements to the hot ranges page. The first is the omission of table descriptors if specified, the second allows callers to specify per-node limits on the number of ranges requested.

Specifying `StatsOnly` on the hot ranges call will cause the call to skip collecting table descriptors to include in the response, which means the call will not be required to read from the keyspace.

The `PerNodeLimit` specifies a local limit for a hot ranges call, so that we only include a number of replicas for each node local call made, (different than the global limit enforced today).

Fixes: #142595
Epic: CRDB-43150

Release note (general change): Allows api callers to specify statistics only and a per-node limit for the hot ranges response.

144188: jsonpath: separate `silent` error and `strict` structural checks r=normanchenn a=normanchenn

The `jsonb_path_*` functions include a `silent` argument, and JSONPath queries have a `strict` mode. Previously, the implementation combined these variables, treating `silent=true` as equivalent to forcing lax mode (`strict=false`).

However, `strict` mode primary handles errors related to structural issues, whereas `silent` controls whether runtime errors encountered during path evaluation should cause the query to fail or are suppressed.

This commit refactors the evaluation context (`jsonpathCtx`) to handle these distinctly.

Epic: None
Release note: None

Co-authored-by: BramGruneir <[email protected]>
Co-authored-by: Bergin Dedej <[email protected]>
Co-authored-by: Brian Dillmann <[email protected]>
Co-authored-by: Norman Chen <[email protected]>

Author: 5 people

docs/generated/http/full.md

@@ -3599,6 +3599,8 @@ of ranges currently considered “hot” by the node(s).
 | page_token | [string](#cockroach.server.serverpb.HotRangesRequest-string) |  |  | [reserved](#support-status) |
 | tenant_id | [string](#cockroach.server.serverpb.HotRangesRequest-string) |  |  | [reserved](#support-status) |
 | nodes | [string](#cockroach.server.serverpb.HotRangesRequest-string) | repeated |  | [reserved](#support-status) |
+| per_node_limit | [int32](#cockroach.server.serverpb.HotRangesRequest-int32) |  | per_node_limit indicates the maximum number of hot ranges to return for each node. If left empty, the default is 128. | [reserved](#support-status) |
+| stats_only | [bool](#cockroach.server.serverpb.HotRangesRequest-bool) |  | stats_only indicates whether to return only the stats for the hot ranges, without pulling descriptor information. | [reserved](#support-status) |
 
 
 

docs/generated/metrics/metrics.html

@@ -1695,8 +1695,12 @@
 <tr><td>APPLICATION</td><td>sql.distsql.service.latency.internal</td><td>Latency of DistSQL request execution (internal queries)</td><td>SQL Internal Statements</td><td>HISTOGRAM</td><td>NANOSECONDS</td><td>AVG</td><td>NONE</td></tr>
 <tr><td>APPLICATION</td><td>sql.distsql.vec.openfds</td><td>Current number of open file descriptors used by vectorized external storage</td><td>Files</td><td>GAUGE</td><td>COUNT</td><td>AVG</td><td>NONE</td></tr>
 <tr><td>APPLICATION</td><td>sql.exec.latency</td><td>Latency of SQL statement execution</td><td>Latency</td><td>HISTOGRAM</td><td>NANOSECONDS</td><td>AVG</td><td>NONE</td></tr>
+<tr><td>APPLICATION</td><td>sql.exec.latency.consistent</td><td>Latency of SQL statement execution of non-historical queries</td><td>Latency</td><td>HISTOGRAM</td><td>NANOSECONDS</td><td>AVG</td><td>NONE</td></tr>
+<tr><td>APPLICATION</td><td>sql.exec.latency.consistent.internal</td><td>Latency of SQL statement execution of non-historical queries (internal queries)</td><td>SQL Internal Statements</td><td>HISTOGRAM</td><td>NANOSECONDS</td><td>AVG</td><td>NONE</td></tr>
 <tr><td>APPLICATION</td><td>sql.exec.latency.detail</td><td>Latency of SQL statement execution, by statement fingerprint</td><td>Latency</td><td>HISTOGRAM</td><td>NANOSECONDS</td><td>AVG</td><td>NONE</td></tr>
 <tr><td>APPLICATION</td><td>sql.exec.latency.detail.internal</td><td>Latency of SQL statement execution, by statement fingerprint (internal queries)</td><td>SQL Internal Statements</td><td>HISTOGRAM</td><td>NANOSECONDS</td><td>AVG</td><td>NONE</td></tr>
+<tr><td>APPLICATION</td><td>sql.exec.latency.historical</td><td>Latency of SQL statement execution of historical queries</td><td>Latency</td><td>HISTOGRAM</td><td>NANOSECONDS</td><td>AVG</td><td>NONE</td></tr>
+<tr><td>APPLICATION</td><td>sql.exec.latency.historical.internal</td><td>Latency of SQL statement execution of historical queries (internal queries)</td><td>SQL Internal Statements</td><td>HISTOGRAM</td><td>NANOSECONDS</td><td>AVG</td><td>NONE</td></tr>
 <tr><td>APPLICATION</td><td>sql.exec.latency.internal</td><td>Latency of SQL statement execution (internal queries)</td><td>SQL Internal Statements</td><td>HISTOGRAM</td><td>NANOSECONDS</td><td>AVG</td><td>NONE</td></tr>
 <tr><td>APPLICATION</td><td>sql.failure.count</td><td>Number of statements resulting in a planning or runtime error</td><td>SQL Statements</td><td>COUNTER</td><td>COUNT</td><td>AVG</td><td>NON_NEGATIVE_DERIVATIVE</td></tr>
 <tr><td>APPLICATION</td><td>sql.failure.count.internal</td><td>Number of statements resulting in a planning or runtime error (internal queries)</td><td>SQL Internal Statements</td><td>COUNTER</td><td>COUNT</td><td>AVG</td><td>NON_NEGATIVE_DERIVATIVE</td></tr>
@@ -1819,6 +1823,10 @@
 <tr><td>APPLICATION</td><td>sql.select.started.count</td><td>Number of SQL SELECT statements started</td><td>SQL Statements</td><td>COUNTER</td><td>COUNT</td><td>AVG</td><td>NON_NEGATIVE_DERIVATIVE</td></tr>
 <tr><td>APPLICATION</td><td>sql.select.started.count.internal</td><td>Number of SQL SELECT statements started (internal queries)</td><td>SQL Internal Statements</td><td>COUNTER</td><td>COUNT</td><td>AVG</td><td>NON_NEGATIVE_DERIVATIVE</td></tr>
 <tr><td>APPLICATION</td><td>sql.service.latency</td><td>Latency of SQL request execution</td><td>Latency</td><td>HISTOGRAM</td><td>NANOSECONDS</td><td>AVG</td><td>NONE</td></tr>
+<tr><td>APPLICATION</td><td>sql.service.latency.consistent</td><td>Latency of SQL request execution of non-historical queries</td><td>Latency</td><td>HISTOGRAM</td><td>NANOSECONDS</td><td>AVG</td><td>NONE</td></tr>
+<tr><td>APPLICATION</td><td>sql.service.latency.consistent.internal</td><td>Latency of SQL request execution of non-historical queries (internal queries)</td><td>SQL Internal Statements</td><td>HISTOGRAM</td><td>NANOSECONDS</td><td>AVG</td><td>NONE</td></tr>
+<tr><td>APPLICATION</td><td>sql.service.latency.historical</td><td>Latency of SQL request execution of historical queries</td><td>Latency</td><td>HISTOGRAM</td><td>NANOSECONDS</td><td>AVG</td><td>NONE</td></tr>
+<tr><td>APPLICATION</td><td>sql.service.latency.historical.internal</td><td>Latency of SQL request execution of historical queries (internal queries)</td><td>SQL Internal Statements</td><td>HISTOGRAM</td><td>NANOSECONDS</td><td>AVG</td><td>NONE</td></tr>
 <tr><td>APPLICATION</td><td>sql.service.latency.internal</td><td>Latency of SQL request execution (internal queries)</td><td>SQL Internal Statements</td><td>HISTOGRAM</td><td>NANOSECONDS</td><td>AVG</td><td>NONE</td></tr>
 <tr><td>APPLICATION</td><td>sql.statement_timeout.count</td><td>Count of statements that failed because they exceeded the statement timeout</td><td>SQL Statements</td><td>COUNTER</td><td>COUNT</td><td>AVG</td><td>NON_NEGATIVE_DERIVATIVE</td></tr>
 <tr><td>APPLICATION</td><td>sql.statement_timeout.count.internal</td><td>Count of statements that failed because they exceeded the statement timeout (internal queries)</td><td>SQL Internal Statements</td><td>COUNTER</td><td>COUNT</td><td>AVG</td><td>NON_NEGATIVE_DERIVATIVE</td></tr>

pkg/cmd/roachtest/tests/nodejs_postgres.go

@@ -150,14 +150,22 @@ PGSSLCERT=$HOME/certs/client.%[1]s.crt PGSSLKEY=$HOME/certs/client.%[1]s.key PGS
 		rawResultsStr := result.Stdout + result.Stderr
 		t.L().Printf("Test Results: %s", rawResultsStr)
 		if err != nil {
-			// The one failing test is `pool size of 1` which
-			// fails because it does SELECT count(*) FROM pg_stat_activity which is
-			// not implemented in CRDB.
-			if strings.Contains(rawResultsStr, "1 failing") &&
-				// Failing tests are listed numerically, we only expect one.
-				// The one failing test should be "pool size of 1".
-				strings.Contains(rawResultsStr, "1) pool size of 1") {
-				err = nil
+			// Check for expected test failures. We allow:
+			// 1. One failing test that is "pool size of 1"
+			// 2. One failing test that is "events"
+			// 3. Two failing tests that are exactly "events" and "pool size of 1"
+			if strings.Contains(rawResultsStr, "1 failing") {
+				// Single test failure case
+				if strings.Contains(rawResultsStr, "1) pool size of 1") ||
+					strings.Contains(rawResultsStr, "1) events") {
+					err = nil
+				}
+			} else if strings.Contains(rawResultsStr, "2 failing") {
+				// Two test failures case - must be exactly events and pool size of 1
+				if strings.Contains(rawResultsStr, "1) events") &&
+					strings.Contains(rawResultsStr, "2) pool size of 1") {
+					err = nil
+				}
 			}
 			if err != nil {
 				t.Fatal(err)

pkg/server/serverpb/status.proto

@@ -1387,6 +1387,18 @@ message HotRangesRequest {
     (gogoproto.customname) = "Nodes",
     (gogoproto.nullable) = true
   ];
+  // per_node_limit indicates the maximum number of hot ranges
+  // to return for each node. If left empty, the default is 128.
+  int32 per_node_limit = 6 [
+    (gogoproto.customname) = "PerNodeLimit",
+    (gogoproto.nullable) = true
+  ];
+  // stats_only indicates whether to return only the stats
+  // for the hot ranges, without pulling descriptor information.
+  bool stats_only = 7 [
+    (gogoproto.customname) = "StatsOnly",
+    (gogoproto.nullable) = true
+  ];
 }
 
 // HotRangesResponseV2 is a response payload returned by `HotRangesV2` service.

pkg/server/status.go

@@ -7,6 +7,7 @@ package server
 
 import (
 	"bytes"
+	"cmp"
 	"context"
 	"crypto/ecdsa"
 	"crypto/rsa"
@@ -19,6 +20,7 @@ import (
 	"os/exec"
 	"reflect"
 	"regexp"
+	"slices"
 	"sort"
 	"strconv"
 	"strings"
@@ -2863,7 +2865,7 @@ func (t *statusServer) HotRangesV2(
 	}
 
 	ti, _ := t.sqlServer.tenantConnect.TenantInfo()
-	if ti.TenantID.IsSet() {
+	if ti.TenantID.IsSet() && !req.StatsOnly {
 		err = t.addDescriptorsToHotRanges(ctx, resp)
 		if err != nil {
 			return nil, err
@@ -2923,13 +2925,13 @@ func (s *systemStatusServer) HotRangesV2(
 				return nil, errors.New("cannot call 'local' mixed with other nodes")
 			}
 
-			resp, err := s.localHotRanges(ctx, tenantID, requestedNodeID)
+			resp, err := s.localHotRanges(tenantID, requestedNodeID, int(req.PerNodeLimit))
 			if err != nil {
 				return nil, err
 			}
 
-			// If operating as the system tenant, add descriptor data to the reposnse.
-			if !tenantID.IsSet() {
+			// If explicitly set as the system tenant, or unset, add descriptor data to the reposnse.
+			if !tenantID.IsSet() && !req.StatsOnly {
 				err = s.addDescriptorsToHotRanges(ctx, resp)
 				if err != nil {
 					return nil, err
@@ -2943,7 +2945,12 @@ func (s *systemStatusServer) HotRangesV2(
 		requestedNodes = append(requestedNodes, requestedNodeID)
 	}
 
-	remoteRequest := serverpb.HotRangesRequest{Nodes: []string{"local"}, TenantID: req.TenantID}
+	remoteRequest := serverpb.HotRangesRequest{
+		Nodes:        []string{"local"},
+		TenantID:     req.TenantID,
+		PerNodeLimit: req.PerNodeLimit,
+		StatsOnly:    req.StatsOnly,
+	}
 	nodeFn := func(ctx context.Context, status serverpb.StatusClient, nodeID roachpb.NodeID) ([]*serverpb.HotRangesResponseV2_HotRange, error) {
 		nodeResp, err := status.HotRangesV2(ctx, &remoteRequest)
 		if err != nil {
@@ -2990,7 +2997,7 @@ func (s *systemStatusServer) HotRangesV2(
 // Returns a HotRangesResponseV2 containing detailed information about each hot range,
 // or an error if the operation fails.
 func (s *systemStatusServer) localHotRanges(
-	ctx context.Context, tenantID roachpb.TenantID, requestedNodeID roachpb.NodeID,
+	tenantID roachpb.TenantID, requestedNodeID roachpb.NodeID, localLimit int,
 ) (*serverpb.HotRangesResponseV2, error) {
 	// Initialize response object
 	var resp serverpb.HotRangesResponseV2
@@ -3048,6 +3055,15 @@ func (s *systemStatusServer) localHotRanges(
 		return nil, err
 	}
 
+	// sort the slices by cpu
+	slices.SortFunc(resp.Ranges, func(a, b *serverpb.HotRangesResponseV2_HotRange) int {
+		return cmp.Compare(a.CPUTimePerSecond, b.CPUTimePerSecond)
+	})
+	// truncate the response if localLimit is set
+	if localLimit != 0 && localLimit < len(resp.Ranges) {
+		resp.Ranges = resp.Ranges[:localLimit]
+	}
+
 	return &resp, nil
 }
 

pkg/server/status_test.go

@@ -8,6 +8,7 @@ package server
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"os"
 	"slices"
 	"sync"
@@ -820,3 +821,101 @@ func TestHotRangesByNode(t *testing.T) {
 		require.Error(t, err, "cannot call 'local' mixed with other nodes")
 	})
 }
+
+func TestHotRangesStatsOnly(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	sc := log.ScopeWithoutShowLogs(t)
+	defer sc.Close(t)
+
+	ctx := context.Background()
+
+	s := serverutils.StartServerOnly(t, base.TestServerArgs{
+		DefaultTestTenant: base.TestControlsTenantsExplicitly,
+		StoreSpecs: []base.StoreSpec{
+			base.DefaultTestStoreSpec,
+			base.DefaultTestStoreSpec,
+			base.DefaultTestStoreSpec,
+		},
+		Knobs: base.TestingKnobs{
+			Store: &kvserver.StoreTestingKnobs{
+				ReplicaPlannerKnobs: plan.ReplicaPlannerTestingKnobs{
+					DisableReplicaRebalancing: true,
+				},
+			},
+		},
+	})
+	defer s.Stopper().Stop(ctx)
+
+	for _, test := range []struct {
+		statsOnly      bool
+		hasDescriptors bool
+	}{
+		{true, false},
+		{false, true},
+	} {
+		t.Run(fmt.Sprintf("statsOnly=%t hasDescriptors %t", test.statsOnly, test.hasDescriptors), func(t *testing.T) {
+			testutils.SucceedsSoon(t, func() error {
+				ss := s.StatusServer().(*systemStatusServer)
+				resp, err := ss.HotRangesV2(ctx, &serverpb.HotRangesRequest{NodeID: "local", StatsOnly: test.statsOnly})
+				if err != nil {
+					return err
+				}
+
+				if len(resp.Ranges) == 0 {
+					return errors.New("waiting for hot ranges to be collected")
+				}
+
+				hasDescriptors := false
+				for _, r := range resp.Ranges {
+					allDescriptors := append(r.Databases, append(r.Tables, r.Indexes...)...)
+					if len(allDescriptors) > 0 {
+						hasDescriptors = true
+					}
+				}
+
+				require.Equal(t, test.hasDescriptors, hasDescriptors)
+				return nil
+			})
+		})
+	}
+}
+
+func TestHotRangesNodeLimit(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	sc := log.ScopeWithoutShowLogs(t)
+	defer sc.Close(t)
+
+	ctx := context.Background()
+
+	s := serverutils.StartServerOnly(t, base.TestServerArgs{
+		DefaultTestTenant: base.TestControlsTenantsExplicitly,
+		StoreSpecs: []base.StoreSpec{
+			base.DefaultTestStoreSpec,
+			base.DefaultTestStoreSpec,
+			base.DefaultTestStoreSpec,
+		},
+		Knobs: base.TestingKnobs{
+			Store: &kvserver.StoreTestingKnobs{
+				ReplicaPlannerKnobs: plan.ReplicaPlannerTestingKnobs{
+					DisableReplicaRebalancing: true,
+				},
+			},
+		},
+	})
+	defer s.Stopper().Stop(ctx)
+
+	testutils.SucceedsSoon(t, func() error {
+		ss := s.StatusServer().(*systemStatusServer)
+		resp, err := ss.HotRangesV2(ctx, &serverpb.HotRangesRequest{NodeID: "local", PerNodeLimit: 5})
+		if err != nil {
+			return err
+		}
+
+		if len(resp.Ranges) == 0 {
+			return errors.New("waiting for hot ranges to be collected")
+		}
+
+		require.Equal(t, 5, len(resp.Ranges))
+		return nil
+	})
+}

pkg/sql/conn_executor.go

@@ -557,6 +557,7 @@ func makeMetrics(internal bool, sv *settings.Values) Metrics {
 			SQLOptPlanCacheMisses:         metric.NewCounter(getMetricMeta(MetaSQLOptPlanCacheMisses, internal)),
 			StatementFingerprintCount:     metric.NewUniqueCounter(getMetricMeta(MetaUniqueStatementCount, internal)),
 			SQLExecLatencyDetail:          sqlExecLatencyDetail,
+
 			// TODO(mrtracy): See HistogramWindowInterval in server/config.go for the 6x factor.
 			DistSQLExecLatency: metric.NewHistogram(metric.HistogramOptions{
 				Mode:         metric.HistogramModePreferHdrLatency,
@@ -570,6 +571,18 @@ func makeMetrics(internal bool, sv *settings.Values) Metrics {
 				Duration:     6 * metricsSampleInterval,
 				BucketConfig: metric.IOLatencyBuckets,
 			}),
+			SQLExecLatencyConsistent: metric.NewHistogram(metric.HistogramOptions{
+				Mode:         metric.HistogramModePreferHdrLatency,
+				Metadata:     getMetricMeta(MetaSQLExecLatencyConsistent, internal),
+				Duration:     6 * metricsSampleInterval,
+				BucketConfig: metric.IOLatencyBuckets,
+			}),
+			SQLExecLatencyHistorical: metric.NewHistogram(metric.HistogramOptions{
+				Mode:         metric.HistogramModePreferHdrLatency,
+				Metadata:     getMetricMeta(MetaSQLExecLatencyHistorical, internal),
+				Duration:     6 * metricsSampleInterval,
+				BucketConfig: metric.IOLatencyBuckets,
+			}),
 			DistSQLServiceLatency: metric.NewHistogram(metric.HistogramOptions{
 				Mode:         metric.HistogramModePreferHdrLatency,
 				Metadata:     getMetricMeta(MetaDistSQLServiceLatency, internal),
@@ -582,6 +595,18 @@ func makeMetrics(internal bool, sv *settings.Values) Metrics {
 				Duration:     6 * metricsSampleInterval,
 				BucketConfig: metric.IOLatencyBuckets,
 			}),
+			SQLServiceLatencyConsistent: metric.NewHistogram(metric.HistogramOptions{
+				Mode:         metric.HistogramModePreferHdrLatency,
+				Metadata:     getMetricMeta(MetaSQLServiceLatencyConsistent, internal),
+				Duration:     6 * metricsSampleInterval,
+				BucketConfig: metric.IOLatencyBuckets,
+			}),
+			SQLServiceLatencyHistorical: metric.NewHistogram(metric.HistogramOptions{
+				Mode:         metric.HistogramModePreferHdrLatency,
+				Metadata:     getMetricMeta(MetaSQLServiceLatencyHistorical, internal),
+				Duration:     6 * metricsSampleInterval,
+				BucketConfig: metric.IOLatencyBuckets,
+			}),
 			SQLTxnLatency: metric.NewHistogram(metric.HistogramOptions{
 				Mode:         metric.HistogramModePreferHdrLatency,
 				Metadata:     getMetricMeta(MetaSQLTxnLatency, internal),

pkg/sql/exec_util.go

@@ -767,6 +767,18 @@ var (
 		Measurement: "Latency",
 		Unit:        metric.Unit_NANOSECONDS,
 	}
+	MetaSQLExecLatencyConsistent = metric.Metadata{
+		Name:        "sql.exec.latency.consistent",
+		Help:        "Latency of SQL statement execution of non-historical queries",
+		Measurement: "Latency",
+		Unit:        metric.Unit_NANOSECONDS,
+	}
+	MetaSQLExecLatencyHistorical = metric.Metadata{
+		Name:        "sql.exec.latency.historical",
+		Help:        "Latency of SQL statement execution of historical queries",
+		Measurement: "Latency",
+		Unit:        metric.Unit_NANOSECONDS,
+	}
 	MetaSQLExecLatencyDetail = metric.Metadata{
 		Name:        "sql.exec.latency.detail",
 		Help:        "Latency of SQL statement execution, by statement fingerprint",
@@ -780,6 +792,18 @@ var (
 		Measurement: "Latency",
 		Unit:        metric.Unit_NANOSECONDS,
 	}
+	MetaSQLServiceLatencyConsistent = metric.Metadata{
+		Name:        "sql.service.latency.consistent",
+		Help:        "Latency of SQL request execution of non-historical queries",
+		Measurement: "Latency",
+		Unit:        metric.Unit_NANOSECONDS,
+	}
+	MetaSQLServiceLatencyHistorical = metric.Metadata{
+		Name:        "sql.service.latency.historical",
+		Help:        "Latency of SQL request execution of historical queries",
+		Measurement: "Latency",
+		Unit:        metric.Unit_NANOSECONDS,
+	}
 	MetaSQLOptPlanCacheHits = metric.Metadata{
 		Name:        "sql.optimizer.plan_cache.hits",
 		Help:        "Number of non-prepared statements for which a cached plan was used",