kv/bulk: track send-wait by store#79612
Conversation
|
This change is |
tbg
left a comment
There was a problem hiding this comment.
Looks good, had a few comments. We could argue about whether it's good for above-KV clients to set the ClientRangeInfo struct, but I think what you've chosen to do is pragmatic. Adding a new top-level bool doesn't really make it any better, if we really wanted to be super clean, then the ClientRangeInfo field wouldn't exist on the datastructures above DistSender, but here it is, so might as well use it.
I also glanced over the other stuff in this PR and it looked good but you'll want someone else to sign off on it.
|
Adding @adityamaru to look at the first, non-kv commit that just combines the two stats structs. |
|
First commit LGTM, thanks for the cleanup it's much more readable. |
502472b to
940adc6
Compare
tbg
left a comment
There was a problem hiding this comment.
Thanks!
Reviewed 2 of 5 files at r1, 6 of 6 files at r3, 9 of 9 files at r4, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @adityamaru)
Release note: none.
Release note: none.
|
TFTR! bors r+ |
|
Build failed (retrying...): |
|
Build succeeded: |
No description provided.