Merge #85599 #85621 #85622 #85625

85599: ui: add MVCC range stats to Db Console r=koorosh a=koorosh

This patch extends node details with `rangeKeyBytes`
and `rangeValueBytes` stats. Also, it adds the same
stats to Advanced Debug > Range Status section.

Release note (ui change): Added "Range Key Bytes" and
"Range Value Bytes" stats on Node details page.

85621: sql: fix recent race on using a random source r=yuzefovich a=yuzefovich

In 9eb5ac3 we introduced a single
random source to the DistSQLPlanner that is used by the PartitionSpans
logic in the multi-tenant setting. However, we missed the fact that that
method can be called concurrently from multiple goroutines which can
lead to a race on the random source. Prior to that commit we were using
a single global random source, so there was a desire to reduce
contention on that. This commit achieves that desire but also fixes the
data race by instantiating a new random source whenever we're creating
a "resolver" function where the source is used.

Fixes: #85508.

Release note: None

85622: sql: release leases before running migrations r=ajwerner a=ajwerner

We fortunately prevent setting cluster settings in a transaction. That means
that we have full control over the access to tables that run during the
transaction which runs a cluster version upgrade. One observation is that
upgrades can take a long time if we leak leases in some way. In #85391 we
add function resolution which results in decriptors now being leased when
you run the following statement because we need to resolve the function:

`SET CLUSTER SETTING version = crdb_internal.node_executable_version()`

Then the test ends up taking 5 minutes +/- 15% for the leases to expire.
The downside here is that you don't get a transactional view of UDFs used
in expressions inside `SET CLUSTER VERSION`. That seems fine to me.

Release note: None

85625: bazel: move `bazel run :gazelle` before `generate-logictest` r=dt a=rickystewart

`gazelle` populates the `BUILD.bazel` files for the `staticcheck`
analyzers, so we want those populated before `generate-logictest`.

Release note: None

Co-authored-by: Andrii Vorobiov <and.vorobiov@gmail.com>
Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
Co-authored-by: Andrew Werner <awerner32@gmail.com>
Co-authored-by: Ricky Stewart <ricky@cockroachlabs.com>

build/bazelutil/bazel-generate.sh

@@ -55,14 +55,14 @@ else
  bazel run pkg/cmd/generate-staticcheck --run_under="cd $PWD && "
 fi
 
+bazel run //:gazelle
+
 if files_unchanged_from_upstream WORKSPACE $(find_relevant ./pkg/sql/logictest/logictestbase -name BUILD.bazel -or -name '*.go') $(find_relevant ./pkg/sql/logictest/testdata -name '*') $(find_relevant ./pkg/sql/sqlitelogictest -name BUILD.bazel -or -name '*.go') $(find_relevant ./pkg/ccl/logictestccl/testdata -name '*') $(find_relevant pkg/sql/opt/exec/execbuilder/testdata -name '*'); then
  echo "Skipping //pkg/cmd/generate-logictest (relevant files are unchanged from upstream)"
 else
  bazel run pkg/cmd/generate-logictest -- -out-dir="$PWD"
 fi
 
-bazel run //:gazelle
-
 if files_unchanged_from_upstream c-deps/archived.bzl c-deps/REPOSITORIES.bzl DEPS.bzl WORKSPACE $(find_relevant ./pkg/cmd/generate-distdir -name BUILD.bazel -or -name '*.go') $(find_relevant ./pkg/build/bazel -name BUILD.bazel -or -name '*.go') $(find_relevant pkg/build/starlarkutil -name BUILD.bazel -or -name '*.go'); then
  echo "Skipping //pkg/cmd/generate-distdir (relevant files are unchanged from upstream)."
 else

pkg/sql/distsql_physical_planner.go

@@ -13,7 +13,6 @@ package sql
 import (
  "context"
  "fmt"
- "math/rand"
  "reflect"
  "sort"
 
@@ -135,8 +134,6 @@ type DistSQLPlanner struct {
  codec keys.SQLCodec
 
  clock *hlc.Clock
-
- rng *rand.Rand
 }
 
 // DistributionType is an enum defining when a plan should be distributed.
@@ -182,7 +179,6 @@ func NewDistSQLPlanner(
  sqlInstanceProvider sqlinstance.Provider,
  clock *hlc.Clock,
 ) *DistSQLPlanner {
- rng, _ := randutil.NewPseudoRand()
  dsp := &DistSQLPlanner{
  planVersion: planVersion,
  st: st,
@@ -203,7 +199,6 @@ func NewDistSQLPlanner(
  sqlInstanceProvider: sqlInstanceProvider,
  codec: codec,
  clock: clock,
- rng: rng,
  }
 
  dsp.parallelLocalScansSem = quotapool.NewIntPool("parallel local scans concurrency",
@@ -1300,6 +1295,7 @@ func (dsp *DistSQLPlanner) makeSQLInstanceIDForKVNodeIDTenantResolver(
  regionToSQLInstanceIDs[region] = instancesInRegion
  }
 
+ rng, _ := randutil.NewPseudoRand()
  if len(regionToSQLInstanceIDs) > 0 {
  // If we were able to determine the region information at least for some
  // instances, use the region-aware resolver.
@@ -1332,7 +1328,7 @@ func (dsp *DistSQLPlanner) makeSQLInstanceIDForKVNodeIDTenantResolver(
  // favor those that are closed to the gateway. However, we need to
  // be careful since non-query code paths (like CDC and BulkIO) do
  // benefit from the even spread of the spans.
- return instancesInRegion[dsp.rng.Intn(len(instancesInRegion))]
+ return instancesInRegion[rng.Intn(len(instancesInRegion))]
  }
  } else {
  // If it just so happens that we couldn't determine the region for all
@@ -1341,7 +1337,7 @@ func (dsp *DistSQLPlanner) makeSQLInstanceIDForKVNodeIDTenantResolver(
  hasLocalitySet = false
  // Randomize the order in which we choose instances so that work is
  // allocated fairly across queries.
- dsp.rng.Shuffle(len(instances), func(i, j int) {
+ rng.Shuffle(len(instances), func(i, j int) {
  instances[i], instances[j] = instances[j], instances[i]
  })
  var i int

pkg/sql/set_cluster_setting.go

@@ -238,6 +238,7 @@ func (n *setClusterSettingNode) startExec(params runParams) error {
  params.p.EvalContext(),
  params.extendedEvalCtx.Codec.ForSystemTenant(),
  params.p.logEvent,
+ params.p.descCollection.ReleaseLeases,
  )
  if err != nil {
  return err
@@ -317,6 +318,7 @@ func writeSettingInternal(
  evalCtx *eval.Context,
  forSystemTenant bool,
  logFn func(context.Context, descpb.ID, eventpb.EventPayload) error,
+ releaseLeases func(context.Context),
 ) (expectedEncodedValue string, err error) {
  err = execCfg.DB.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error {
  var reportedValue string
@@ -336,6 +338,7 @@ func writeSettingInternal(
  reportedValue, expectedEncodedValue, err = writeNonDefaultSettingValue(
  ctx, execCfg, setting, name, txn,
  user, st, value, forSystemTenant,
+ releaseLeases,
  )
  if err != nil {
  return err
@@ -384,6 +387,7 @@ func writeNonDefaultSettingValue(
  st *cluster.Settings,
  value tree.Datum,
  forSystemTenant bool,
+ releaseLeases func(context.Context),
 ) (reportedValue string, expectedEncodedValue string, err error) {
  // Stringify the value set by the statement for reporting in errors, logs etc.
  reportedValue = tree.AsStringWithFlags(value, tree.FmtBareStrings)
@@ -398,7 +402,9 @@ func writeNonDefaultSettingValue(
  verSetting, isSetVersion := setting.(*settings.VersionSetting)
  if isSetVersion {
  if err := setVersionSetting(
- ctx, execCfg, verSetting, name, txn, user, st, value, encoded, forSystemTenant); err != nil {
+ ctx, execCfg, verSetting, name, txn, user, st, value, encoded,
+ forSystemTenant, releaseLeases,
+ ); err != nil {
  return reportedValue, expectedEncodedValue, err
  }
  } else {
@@ -438,6 +444,7 @@ func setVersionSetting(
  value tree.Datum,
  encoded string,
  forSystemTenant bool,
+ releaseLeases func(context.Context),
 ) error {
  // In the special case of the 'version' cluster setting,
  // we must first read the previous value to validate that the
@@ -534,6 +541,12 @@ func setVersionSetting(
  })
  }
 
+ // If we're here, we know we aren't in a transaction because we don't
+ // allow setting cluster settings in a transaction. We need to release
+ // our leases because the underlying migrations may affect descriptors
+ // we currently have a lease for. We don't need to hold on to any leases
+ // because the code isn't relying on them.
+ releaseLeases(ctx)
  return runMigrationsAndUpgradeVersion(
  ctx, execCfg, user, prev, value, updateVersionSystemSetting,
  )

pkg/ui/workspaces/db-console/src/util/proto.ts

@@ -64,6 +64,8 @@ export namespace MetricConstants {
  export const liveBytes: string = "livebytes";
  export const keyBytes: string = "keybytes";
  export const valBytes: string = "valbytes";
+ export const rangeKeyBytes: string = "rangekeybytes";
+ export const rangeValBytes: string = "rangevalbytes";
  export const totalBytes: string = "totalbytes";
  export const intentBytes: string = "intentbytes";
  export const liveCount: string = "livecount";

pkg/ui/workspaces/db-console/src/views/cluster/containers/nodeOverview/index.tsx

@@ -44,6 +44,8 @@ import {
  NodeUsedCapacityTooltip,
  NodeAvailableCapacityTooltip,
  NodeMaximumCapacityTooltip,
+ MVCCRangeKeyBytesTooltip,
+ MVCCRangeValueBytesTooltip,
 } from "./tooltips";
 import { TooltipProps } from "src/components/tooltip/tooltip";
 
@@ -187,6 +189,24 @@ export class NodeOverview extends React.Component<NodeOverviewProps, {}> {
  nodeName={nodesSummary.nodeDisplayNameByID[node.desc.node_id]}
  CellTooltip={ValueBytesTooltip}
  />
+ <TableRow
+ data={node}
+ title="MVCC Range Key Bytes"
+ valueFn={metrics =>
+ Bytes(metrics[MetricConstants.rangeKeyBytes] || 0)
+ }
+ nodeName={nodesSummary.nodeDisplayNameByID[node.desc.node_id]}
+ CellTooltip={MVCCRangeKeyBytesTooltip}
+ />
+ <TableRow
+ data={node}
+ title="MVCC Range Value Bytes"
+ valueFn={metrics =>
+ Bytes(metrics[MetricConstants.rangeValBytes] || 0)
+ }
+ nodeName={nodesSummary.nodeDisplayNameByID[node.desc.node_id]}
+ CellTooltip={MVCCRangeValueBytesTooltip}
+ />
  <TableRow
  data={node}
  title="Intent Bytes"

pkg/ui/workspaces/db-console/src/views/cluster/containers/nodeOverview/tooltips.tsx

@@ -85,6 +85,48 @@ export const ValueBytesTooltip: React.FC<
  </Tooltip>
 );
 
+export const MVCCRangeKeyBytesTooltip: React.FC<
+ TooltipProps & {
+ nodeName: string;
+ }
+> = props => (
+ <Tooltip
+ {...props}
+ placement="bottom"
+ title={
+ <div className="tooltip__table--title">
+ <p>
+ Number of bytes stored in MVCC range keys on node{" "}
+ {props.nodeName || "NodeName"}.
+ </p>
+ </div>
+ }
+ >
+ {props.children}
+ </Tooltip>
+);
+
+export const MVCCRangeValueBytesTooltip: React.FC<
+ TooltipProps & {
+ nodeName: string;
+ }
+> = props => (
+ <Tooltip
+ {...props}
+ placement="bottom"
+ title={
+ <div className="tooltip__table--title">
+ <p>
+ Number of bytes stored in MVCC range values on node{" "}
+ {props.nodeName || "NodeName"}.
+ </p>
+ </div>
+ }
+ >
+ {props.children}
+ </Tooltip>
+);
+
 export const IntentBytesTooltip: React.FC<TooltipProps> = props => (
  <Tooltip
  {...props}

pkg/ui/workspaces/db-console/src/views/reports/containers/range/rangeTable.tsx

@@ -170,6 +170,16 @@ const rangeTableDisplayList: RangeTableRow[] = [
  display: "MVCC Value Bytes/Count",
  compareToLeader: true,
  },
+ {
+ variable: "mvccRangeKeyBytesCount",
+ display: "MVCC Range Key Bytes/Count",
+ compareToLeader: true,
+ },
+ {
+ variable: "mvccRangeValueBytesCount",
+ display: "MVCC Range Value Bytes/Count",
+ compareToLeader: true,
+ },
  {
  variable: "mvccIntentBytesCount",
  display: "MVCC Intent Bytes/Count",
@@ -796,6 +806,14 @@ export default class RangeTable extends React.Component<RangeTableProps, {}> {
  FixLong(mvcc.val_bytes),
  FixLong(mvcc.val_count),
  ),
+ mvccRangeKeyBytesCount: this.contentMVCC(
+ FixLong(mvcc.range_key_bytes || 0),
+ FixLong(mvcc.range_key_count || 0),
+ ),
+ mvccRangeValueBytesCount: this.contentMVCC(
+ FixLong(mvcc.range_val_bytes || 0),
+ FixLong(mvcc.range_val_count || 0),
+ ),
  mvccIntentBytesCount: this.contentMVCC(
  FixLong(mvcc.intent_bytes),
  FixLong(mvcc.intent_count),