-
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
kvclient: remove RPCContext from DistSender #116644
kvclient: remove RPCContext from DistSender #116644
Conversation
1cc3c60
to
dd4ef8a
Compare
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.
Reviewed 12 of 12 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @andrewbaptist, @arulajmani, @dhartunian, and @nvanbenschoten)
pkg/kv/kvclient/kvcoord/dist_sender.go
line 668 at r1 (raw file):
healthFunc: cfg.HealthFunc, latencyFunc: cfg.LatencyFunc, stopper: cfg.Stopper,
nit: maybe move stopper to be after st
?
pkg/kv/kvserver/store_test.go
line 259 at r1 (raw file):
Clock: cfg.Clock, NodeDescs: mockNodeStore{desc: nodeDesc},
nit: stray new line.
dd4ef8a
to
692d4f8
Compare
bors r=yuzefovich TFTR! (and the really quick turnaround) |
RPCContext is unnecessary in DistSender. The only two things that are needed are the LatencyFunc, which is extracted from the RemoteClocks, and the Stopper which should have been passed in directly. This change makes DistSender a tiny bit more easily testable. Epic: none Release note: None
692d4f8
to
de430df
Compare
Canceled. |
bors retry Fixed the incorrect spacing from the merge conflict. |
Build succeeded: |
RPCContext is unnecessary in DistSender. The only two things that are needed are the LatencyFunc, which is extracted from the RemoteClocks, and the Stopper which should have been passed in directly. This change makes DistSender a tiny bit more easily testable.
Epic: none
Release note: None