sql: Add range size to SHOW RANGES command.

[rohany]

Fixes #39767.

This is done by adding a new builtin function crdb_internal.range_stats that returns a json object containing stats about the requested range.

Release note (sql change): Add range size information to SHOW RANGES. Adds a builtin function crdb_internal.range_stats which returns a json object containing stats about the requested range.

[cockroach-teamcity]

This change is 

[nvanbenschoten]

Thanks for getting this out so fast @rohany. There are a few things here that we'll likely want to change before merging this.

The first is that we almost certainly don't want to include this information in crdb_internal.ranges_no_leases. The reason for the split between crdb_internal.ranges_no_leases and crdb_internal.ranges is because crdb_internal.ranges is so much more expensive due to its per-range lookups. We created crdb_internal.ranges_no_leases as a means of quickly accessing the state about ranges that can be gathered exclusively from a meta keyspace scan so that we could continue to expose something relatively fast. This PR is regressing on this front. To avoid this, we'll only want to add this new column to crdb_internal.ranges.

Similarly, it doesn't seem particularly desirable to have the logic for retrieving these stats locked up inside of the virtual schema generator function. I would have expected us to introduce a builtin that mirrors crdb_internal.lease_holder and use that from crdb_internal.ranges. In fact, introducing a crdb_internal.range_stats builtin function that returns a JSON blob of a range's stats seems like a generally useful change in and of itself. We would then use this function from the crdb_internal.ranges view just like we use crdb_internal.lease_holder. Did you consider implementing this PR like that?

@jordanlewis should take a look at this as well, since he reviewed #30601.

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @nvanbenschoten and @rohany)

---

pkg/sql/crdb_internal.go, line 1874 at r1 (raw file):

	split_enforced_until,
	crdb_internal.lease_holder(start_key) AS lease_holder,
  range_size

Mind fixing this spacing?

---

pkg/sql/crdb_internal.go, line 1910 at r1 (raw file):

  table_name           STRING NOT NULL,
  index_name           STRING NOT NULL,
	replicas             INT[] NOT NULL,

Mind fixing this spacing?

[rohany]

I can try implementing it like that via a built in, but I thought we wanted to batch the requests more, rather than doing individual lookups.

However it should be an easy change to expose a new built in function.

Re the spacing: it always looks fine on my editor....

[nvanbenschoten]

I can try implementing it like that via a built in, but I thought we wanted to batch the requests more, rather than doing individual lookups.

Yeah, batching would be ideal, but we're already not batching the crdb_internal.lease_holder lookups, so I don't think it's worth going too far out of the way to batch more. Also, these requests are all going to different ranges, so the only effect of batching is that it allows DistSender to parallelize lookups. It doesn't actually save on RPCs.

Re the spacing: it always looks fine on my editor....

Maybe it's a tabs vs. spaces thing?

[rohany]

RFAL

[nvanbenschoten]

Reviewed 1 of 5 files at r2.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis, @nvanbenschoten, and @rohany)

---

pkg/sql/sem/builtins/builtins.go, line 3084 at r2 (raw file):

				{"key", types.Bytes},
			},
			ReturnType: tree.FixedReturnType(types.Int),

This would be even more useful if we made it crdb_internal.range_stats and it returned a JSON type that included every field in the MVCCStats. You could then pull our the size from the JSON datum. Are there any blockers to doing that?

By the way, I think you want Total() (KeyBytes + ValBytes).

[rohany]

Yeah, that makes sense -- sorry for not doing it the first time.

[rohany]

RFAL

[nvanbenschoten]

Reviewed 1 of 3 files at r3.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis, @nvanbenschoten, and @rohany)

---

pkg/sql/sem/builtins/builtins.go, line 3098 at r3 (raw file):

				}
				resp := b.RawResponse().Responses[0].GetInner().(*roachpb.RangeStatsResponse).MVCCStats
				jsonStr, err := gojson.Marshal(&resp)

@justinj do you mind confirming that this is the best way to do this conversion?

---

pkg/sql/sem/builtins/builtins.go, line 3108 at r3 (raw file):

				return jsonDatum, nil
			},
			Info: "This function is used to retrieve the size of a range in bytes.",

This info looks stale.

[justinj]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis, @justinj, @nvanbenschoten, and @rohany)

---

pkg/sql/sem/builtins/builtins.go, line 3098 at r3 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

@justinj do you mind confirming that this is the best way to do this conversion?

Seems fine to me, if a little fragile? I guess it depends if you want this to be automatically updated if MVCCStats changes. the method lgtm

[nvanbenschoten]

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @jordanlewis, @nvanbenschoten, and @rohany)

---

pkg/sql/sem/builtins/builtins.go, line 3108 at r3 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

This info looks stale.

Statistics, not status.

[rohany]

Sorry about that.

TFTR!

bors r+

[craig]

Build succeeded

• GitHub CI (Cockroach)