Merge #157160

157160: sql: improve observability for statements with hints r=DrewKimball a=DrewKimball

#### builtins: decrease wait time for await_statement_hints_cache

Previously, `crdb_internal.await_statement_hints_cache` waited until
the hint cache frontier timestamp reached `now()` plus the closed
timestamp target duration and refresh interval. The frontier timestamp
indicates that we've seen all events up to that timestamp, so it's OK
to just wait until `now()` instead.

Epic: None

Release note: None

#### explain: display number of hints for EXPLAIN ANALYZE

This commit adds a top-level field to the `EXPLAIN ANALYZE` output
showing the number of hints applied to the statement if nonzero.

Informs #121502

Release note (sql change): EXPLAIN ANALYZE will now display the number
of hints from `system.statement_hints` applied to the executed statement.

#### ui: show whether statement hints were applied in DB console

This commit adds a field to the "plan details" shown for statements in
the "statement activity" page of the DB console. The new field indicates
whether any statement hints from `system.statement_hints` were applied
to the statement.

Informs #121502

Release note (sql change): Plan details in the "statement activity" page
of the DB console now show whether any hints from `system.statement_hints`
werer applied to the statement execution.

Co-authored-by: Drew Kimball <drewk@cockroachlabs.com>

Author: craig[bot]

pkg/sql/appstatspb/app_stats.go

@@ -214,6 +214,7 @@ func (s *StatementStatistics) Add(other *StatementStatistics) {
 	s.Count += other.Count
 	s.FailureCount += other.FailureCount
 	s.GenericCount += other.GenericCount
+	s.StmtHintsCount += other.StmtHintsCount
 }
 
 // AlmostEqual compares two StatementStatistics and their contained NumericStats

pkg/sql/appstatspb/app_stats.proto

@@ -138,6 +138,10 @@ message StatementStatistics {
   // generic_count is the count of executions that used a generic query plan.
   optional int64 generic_count = 36 [(gogoproto.nullable) = false];
 
+  // stmt_hints_count is the count of executions that applied a hint from the
+  // system.statement_hints table.
+  optional int64 stmt_hints_count = 37 [(gogoproto.nullable) = false];
+
   // Note: be sure to update `sql/app_stats.go` when adding/removing fields here!
 
   reserved 13, 14, 17, 18, 19, 20;

pkg/sql/executor_statement_metrics.go

@@ -225,6 +225,7 @@ func (ex *connExecutor) recordStatementSummary(
 			FingerprintID:        stmtFingerprintID,
 			QuerySummary:         stmt.StmtSummary,
 			Generic:              flags.IsSet(planFlagGeneric),
+			AppliedStmtHints:     len(stmt.Hints) > 0,
 			DistSQL:              flags.ShouldBeDistributed(),
 			Vec:                  flags.IsSet(planFlagVectorized),
 			ImplicitTxn:          implicitTxn,

pkg/sql/explain_plan.go

@@ -120,6 +120,10 @@ func (e *explainPlanNode) startExec(params runParams) error {
 			}
 		}
 
+		if len(params.p.stmt.Hints) > 0 {
+			ob.AddStmtHintCount(uint64(len(params.p.stmt.Hints)))
+		}
+
 		if e.options.Flags[tree.ExplainFlagJSON] {
 			// For the JSON flag, we only want to emit the diagram JSON.
 			rows = []string{diagramJSON}

pkg/sql/faketreeeval/BUILD.bazel

@@ -10,7 +10,6 @@ go_library(
         "//pkg/jobs/jobspb",
         "//pkg/roachpb",
         "//pkg/security/username",
-        "//pkg/settings/cluster",
         "//pkg/sql/catalog/descpb",
         "//pkg/sql/hintpb",
         "//pkg/sql/pgwire/pgcode",

pkg/sql/faketreeeval/evalctx.go

@@ -14,7 +14,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/security/username"
-	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/hintpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
@@ -583,7 +582,7 @@ func (ep *DummyEvalPlanner) ClearTableStatsCache() {}
 func (ep *DummyEvalPlanner) ClearStatementHintsCache() {}
 
 // AwaitStatementHintsCache is part of the eval.Planner interface.
-func (ep *DummyEvalPlanner) AwaitStatementHintsCache(ctx context.Context, st *cluster.Settings) {}
+func (ep *DummyEvalPlanner) AwaitStatementHintsCache(ctx context.Context) {}
 
 // RetryCounter is part of the eval.Planner interface.
 func (ep *DummyEvalPlanner) RetryCounter() int {

pkg/sql/hints/BUILD.bazel

@@ -14,8 +14,6 @@ go_library(
         "//pkg/kv/kvclient/rangefeed/rangefeedbuffer",
         "//pkg/kv/kvclient/rangefeed/rangefeedcache",
         "//pkg/kv/kvpb",
-        "//pkg/kv/kvserver",
-        "//pkg/kv/kvserver/closedts",
         "//pkg/roachpb",
         "//pkg/settings",
         "//pkg/settings/cluster",

pkg/sql/hints/hint_cache.go

@@ -17,8 +17,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/kv/kvclient/rangefeed/rangefeedbuffer"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvclient/rangefeed/rangefeedcache"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
-	"github.com/cockroachdb/cockroach/pkg/kv/kvserver"
-	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/closedts"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/settings"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
@@ -381,22 +379,18 @@ var _ rangefeedbuffer.Event = &bufferEvent{}
 // with the present. After Await returns, MaybeGetStatementHints should
 // accurately reflect all hints that were modified before the call to Await
 // (assuming the ctx was not canceled).
-func (c *StatementHintsCache) Await(ctx context.Context, st *cluster.Settings) {
-	// The frontier timestamp comes from the rangefeed, and could be up to
-	// kv.closed_timestamp.target_duration +
-	// kv.rangefeed.closed_timestamp_refresh_interval behind the present.
-	targetDuration := closedts.TargetDuration.Get(&st.SV)
-	refreshInterval := kvserver.RangeFeedRefreshInterval.Get(&st.SV)
-	const fudge = 10 * time.Millisecond
-	waitUntil := c.clock.Now().AddDuration(targetDuration + refreshInterval + fudge).WallTime
+func (c *StatementHintsCache) Await(ctx context.Context) {
+	// Wait in intervals of at least 100 milliseconds to avoid busy-waiting.
+	const minWait = time.Millisecond * 100
+	waitUntil := c.clock.Now().WallTime
 
 	// Await is only used for testing, so we don't need to wake up immediately. We
 	// can get away with polling the frontier time.
-	for frontier := c.frontier.Load(); frontier < waitUntil; frontier = c.frontier.Load() {
+	for frontier := c.frontier.Load(); frontier <= waitUntil; frontier = c.frontier.Load() {
 		select {
 		case <-ctx.Done():
 			return
-		case <-time.After(time.Duration(waitUntil-frontier) * time.Nanosecond):
+		case <-time.After(max(time.Duration(waitUntil-frontier), minWait)):
 		}
 	}
 }

pkg/sql/instrumentation.go

@@ -207,6 +207,10 @@ type instrumentationHelper struct {
 	// stats scanned by this query.
 	nanosSinceStatsForecasted time.Duration
 
+	// stmtHintsCount is the number of hints from system.statement_hints applied
+	// to the statement.
+	stmtHintsCount uint64
+
 	// retryCount is the number of times the transaction was retried.
 	retryCount uint64
 
@@ -429,6 +433,7 @@ func (ih *instrumentationHelper) Setup(
 	ih.implicitTxn = implicitTxn
 	ih.txnPriority = txnPriority
 	ih.txnBufferedWritesEnabled = p.txn.BufferedWritesEnabled()
+	ih.stmtHintsCount = uint64(len(stmt.Hints))
 	ih.retryCount = uint64(retryCount)
 	ih.codec = cfg.Codec
 	ih.origCtx = ctx
@@ -861,6 +866,7 @@ func (ih *instrumentationHelper) emitExplainAnalyzePlanToOutputBuilder(
 	ob.AddDistribution(ih.distribution.String())
 	ob.AddVectorized(ih.vectorized)
 	ob.AddPlanType(ih.generic, ih.optimized)
+	ob.AddStmtHintCount(ih.stmtHintsCount)
 	ob.AddRetryCount("transaction", ih.retryCount)
 	ob.AddRetryTime("transaction", phaseTimes.GetTransactionRetryLatency())
 	ob.AddRetryCount("statement", ih.retryStmtCount)

pkg/sql/logictest/testdata/logic_test/statement_hint_builtins

@@ -1,4 +1,5 @@
 # LogicTest: !local-mixed-25.2 !local-mixed-25.3 !local-prepared
+# cluster-opt: disable-mvcc-range-tombstones-for-point-deletes
 
 statement ok
 CREATE TABLE xy (x INT PRIMARY KEY, y INT, INDEX (y));