-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
server: add StoreIDs
to SpanStats response
#130049
Conversation
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. |
This commit adds a new response field to the SpanStats rpc, `StoreIDs` which is a list of unique store ids in ascending order that store the requestd span. Fixes: cockroachdb#129060 Release note: None
f09d80e
to
fbd2ae6
Compare
// Verify stats across different spans. | ||
for _, tcase := range testCases { | ||
rSpan, err := keys.SpanAddr(tcase.span) | ||
require.NoError(t, err) | ||
|
||
// Assert expected values from multi-span request | ||
spanStats := multiResult.SpanToStats[tcase.span.String()] | ||
if !equalStoreIDs([]roachpb.StoreID{1, 2, 3}, spanStats.StoreIDs) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, great test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @xinhaoz)
TFTR! |
This commit adds a new response field to the
SpanStats rpc,
StoreIDs
which is a list ofunique store ids in ascending order that store
the requested span.
Fixes: #129060
Release note: None