-
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
raft: remove unused read-only requests #120613
Conversation
5cdef50
to
8a367c9
Compare
fc2b600
to
449f9d1
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 14 of 14 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @pav-kv)
-- commits
line 8 at r2:
Does this allow us to delete the Context
field from the Message
proto?
pkg/raft/raftpb/raft.proto
line 69 at r1 (raw file):
// raft/util_test.go. reserved 15, 16; // used to be MsgReadIndex(Resp)
nit: tab to align with above.
Epic: none Release note: none
Epic: none Release note: none
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! 0 of 0 LGTMs obtained (waiting on @pav-kv)
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
Does this allow us to delete the
Context
field from theMessage
proto?
This field is still used by other bits, e.g. by leader transfers:
Line 1054 in e89a1a6
r.send(pb.Message{To: id, Term: term, Type: voteMsg, Index: last.index, LogTerm: last.term, Context: ctx}) |
pkg/raft/raftpb/raft.proto
line 69 at r1 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
nit: tab to align with above.
Done.
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 1 of 2 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @pav-kv)
bors r=nvanbenschoten |
This PR removes the read-only requests from
pkg/raft
. We don't use them in CRDB, and the leases implementation is known to be incorrect (e.g. see etcd-io/raft#166 and etcd-io/etcd#10082).Epic: none
Release note: none