server: return authoritative span statistics for db details endpoint

[THardy98]

Resolves: #96163

This change makes the admin API endpoint getting database statistics scan KV for span statistics instead of using the range descriptor cache. This provides authoritative output, helping deflake TestMultiRegionDatabaseStats.

Release note (sql change): admin API database details endpoint now returns authoritative range statistics.

[blathers-crl]

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

[cockroach-teamcity]

This change is 

[j82w]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @THardy98)

---

-- commits line 12 at r1:
Is there any concern about the additional overhead and perf impact of not using the cache? Would it not be better to just reset the stats for the test?

[THardy98]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @j82w)

---

-- commits line 12 at r1:

Previously, j82w (Jake) wrote…

Is there any concern about the additional overhead and perf impact of not using the cache? Would it not be better to just reset the stats for the test?

I think this makes more sense for the endpoint generally. Using the descriptor cache to measure size seems odd. There's no guarantee the cache holds all the database/table's ranges (i.e. we could be missing ranges or reading old ranges that no longer correspond to the database/table) and there's also no guarantee that the cache at some point will be up-to-date either.

I do agree that there is a concern about the additional overhead/perf impact but we also use the same logic already for the status server which we use for the console. I think the alignment between the two is also good.

[knz]

I agree with Thomas.

Additionally, if we find that performance is a concern, I would welcome a caching layer for the computed stats in the o11y service, separate from the range cache.

[maryliag]

Just wanna see that the tests are passing before approval, to make sure this change is working as expected, since some of the failed ones are related to retrieving database details

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @j82w)

[THardy98]

Tests were failing due to directly accessing meta2 keyspace which would fail in tenant scenarios.

Added a conditional to use the range iterator from tenantConnect if it was populated. Not sure if the tenantConnect != nil pattern is one we want to adopt but thought it was a simple solution here.

[maryliag]

Reviewed 1 of 2 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @j82w)

[THardy98]

bors r+

[craig]

Build failed (retrying...):

• Bazel Essential CI (Cockroach)

[craig]

Build failed (retrying...):

• Bazel Essential CI (Cockroach)

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)

[blathers-crl]

Encountered an error creating backports. Some common things that can go wrong:

1. The backport branch might have already existed.
2. There was a merge conflict.
3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.

---

error creating merge commit from b2faa2b to blathers/backport-release-23.1-108037: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 23.1.x failed. See errors above.

---

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}