server: make SpanStats authoritative#104423
Conversation
|
This change is |
knz
left a comment
There was a problem hiding this comment.
Great work. Thanks for this change. Don't forget to update the test expectation in TestAdminAPITableStats.
Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @zachlite)
-- commits line 8 at r1:
tip: add a blank line after the colon and in-between the two list items to fix the markdown formatting. (You can also upgrade the PR description after that)
32ab463 to
2359c2a
Compare
This commit guarantees that SpanStats uses up-to-date range descriptors on all nodes. The dependency on DistSender is removed and is replaced with a dependency on rangedesc.Scanner. rangedesc.Scanner is used to: 1) Locate nodes that house replicas of a span to avoid cluster-wide fan-outs. 2) Find Range Descriptors that touch a span, to build a SpanStatsResponse. This commit also fixes cockroachdb#103809, where a SpanStatsResponse incorrectly returned the replica_count for a span, instead of the range_count. Epic: https://cockroachlabs.atlassian.net/browse/CRDB-24928 Issue: cockroachdb#103957 Release note (bug fix): SpanStats is no longer subject to stale information, and should be considered authoritative.
2359c2a to
455148e
Compare
arulajmani
left a comment
There was a problem hiding this comment.
Reviewed 1 of 5 files at r1, 3 of 5 files at r2, all commit messages.
Reviewable status:
-- commits line 6 at r2:
To confirm, this only runs on the system tenant, right?
If not, we might need to use a rangedesc.Iterator instead, much like crdb_internal.ranges_no_leases does.
arulajmani
left a comment
There was a problem hiding this comment.
Didn't mean to publish just yet. Change looks good, just a question in-line.
Reviewable status:
zachlite
left a comment
There was a problem hiding this comment.
Yep, span stats are fulfilled by the system tenant (KV nodes). Other tenant requests for span stats are forwarded through tenantConnect.
TFTR
Reviewable status:
|
bors r+ |
|
Build succeeded: |
|
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error setting reviewers, but backport branch blathers/backport-release-23.1-104423 is ready: POST https://api.github.com/repos/cockroachdb/cockroach/pulls/105000/requested_reviewers: 422 Reviews may only be requested from collaborators. One or more of the teams you specified is not a collaborator of the cockroachdb/cockroach repository. [] Backport to branch 23.1.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
This commit guarantees that SpanStats uses up-to-date range
descriptors on all nodes. The dependency on DistSender is removed
and is replaced with a dependency on rangedesc.Scanner.
rangedesc.Scanner is used to:
This commit also fixes #103809, where a SpanStatsResponse incorrectly returned
the replica_count for a span, instead of the range_count.
Epic: https://cockroachlabs.atlassian.net/browse/CRDB-24928
Issue: #103957
Release note (bug fix): SpanStats is no longer subject to stale
information, and should be considered authoritative.