Skip to content
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

Merged
merged 2 commits into from
Apr 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 2 additions & 9 deletions pkg/kv/kvserver/replica_raft.go
Original file line number Diff line number Diff line change
Expand Up @@ -1207,12 +1207,6 @@ type asyncReady struct {
// It is not required to consume or store SoftState.
*raft.SoftState

// ReadStates can be used for node to serve linearizable read requests locally
// when its applied index is greater than the index in ReadState.
// Note that the readState will be returned when raft receives msgReadIndex.
// The returned is only valid for the request that requested to read.
ReadStates []raft.ReadState

// Messages specifies outbound messages to other peers and to local storage
// threads. These messages can be sent in any order.
//
Expand All @@ -1224,9 +1218,8 @@ type asyncReady struct {
// makeAsyncReady constructs an asyncReady from the provided Ready.
func makeAsyncReady(rd raft.Ready) asyncReady {
return asyncReady{
SoftState: rd.SoftState,
ReadStates: rd.ReadStates,
Messages: rd.Messages,
SoftState: rd.SoftState,
Messages: rd.Messages,
}
}

Expand Down
1 change: 0 additions & 1 deletion pkg/raft/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ go_library(
"node.go",
"raft.go",
"rawnode.go",
"read_only.go",
"status.go",
"storage.go",
"types.go",
Expand Down
21 changes: 0 additions & 21 deletions pkg/raft/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,6 @@ type Ready struct {
// Messages slice.
pb.HardState

// ReadStates can be used for node to serve linearizable read requests locally
// when its applied index is greater than the index in ReadState.
// Note that the readState will be returned when raft receives msgReadIndex.
// The returned is only valid for the request that requested to read.
ReadStates []ReadState

// Entries specifies entries to be saved to stable storage BEFORE
// Messages are sent.
//
Expand Down Expand Up @@ -213,19 +207,8 @@ type Node interface {
// from the leader recently). However, 3 can not campaign unilaterally, a
// quorum have to agree that the leader is dead, which avoids disrupting the
// leader if individual nodes are wrong about it being dead.
//
// This does nothing with ReadOnlyLeaseBased, since it would allow a new
// leader to be elected without the old leader knowing.
ForgetLeader(ctx context.Context) error

// ReadIndex request a read state. The read state will be set in the ready.
// Read state has a read index. Once the application advances further than the read
// index, any linearizable read requests issued before the read request can be
// processed safely. The read state will have the same rctx attached.
// Note that request can be lost without notice, therefore it is user's job
// to ensure read index retries.
ReadIndex(ctx context.Context, rctx []byte) error

// Status returns the current status of the raft state machine.
Status() Status
// ReportUnreachable reports the given node is not reachable for the last send.
Expand Down Expand Up @@ -607,7 +590,3 @@ func (n *node) TransferLeadership(ctx context.Context, lead, transferee uint64)
func (n *node) ForgetLeader(ctx context.Context) error {
return n.step(ctx, pb.Message{Type: pb.MsgForgetLeader})
}

func (n *node) ReadIndex(ctx context.Context, rctx []byte) error {
return n.step(ctx, pb.Message{Type: pb.MsgReadIndex, Entries: []pb.Entry{{Data: rctx}}})
}
45 changes: 0 additions & 45 deletions pkg/raft/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,51 +193,6 @@ func TestDisableProposalForwarding(t *testing.T) {
require.Empty(t, r3.msgs)
}

// TestNodeReadIndexToOldLeader ensures that raftpb.MsgReadIndex to old leader
// gets forwarded to the new leader and 'send' method does not attach its term.
func TestNodeReadIndexToOldLeader(t *testing.T) {
r1 := newTestRaft(1, 10, 1, newTestMemoryStorage(withPeers(1, 2, 3)))
r2 := newTestRaft(2, 10, 1, newTestMemoryStorage(withPeers(1, 2, 3)))
r3 := newTestRaft(3, 10, 1, newTestMemoryStorage(withPeers(1, 2, 3)))

nt := newNetwork(r1, r2, r3)

// elect r1 as leader
nt.send(raftpb.Message{From: 1, To: 1, Type: raftpb.MsgHup})

var testEntries = []raftpb.Entry{{Data: []byte("testdata")}}

// send readindex request to r2(follower)
r2.Step(raftpb.Message{From: 2, To: 2, Type: raftpb.MsgReadIndex, Entries: testEntries})

// verify r2(follower) forwards this message to r1(leader) with term not set
require.Len(t, r2.msgs, 1)
readIndxMsg1 := raftpb.Message{From: 2, To: 1, Type: raftpb.MsgReadIndex, Entries: testEntries}
require.Equal(t, readIndxMsg1, r2.msgs[0])

// send readindex request to r3(follower)
r3.Step(raftpb.Message{From: 3, To: 3, Type: raftpb.MsgReadIndex, Entries: testEntries})

// verify r3(follower) forwards this message to r1(leader) with term not set as well.
require.Len(t, r3.msgs, 1)
readIndxMsg2 := raftpb.Message{From: 3, To: 1, Type: raftpb.MsgReadIndex, Entries: testEntries}
require.Equal(t, readIndxMsg2, r3.msgs[0])

// now elect r3 as leader
nt.send(raftpb.Message{From: 3, To: 3, Type: raftpb.MsgHup})

// let r1 steps the two messages previously we got from r2, r3
r1.Step(readIndxMsg1)
r1.Step(readIndxMsg2)

// verify r1(follower) forwards these messages again to r3(new leader)
require.Len(t, r1.msgs, 2)
readIndxMsg3 := raftpb.Message{From: 2, To: 3, Type: raftpb.MsgReadIndex, Entries: testEntries}
require.Equal(t, readIndxMsg3, r1.msgs[0])
readIndxMsg3 = raftpb.Message{From: 3, To: 3, Type: raftpb.MsgReadIndex, Entries: testEntries}
require.Equal(t, readIndxMsg3, r1.msgs[1])
}

// TestNodeProposeConfig ensures that node.ProposeConfChange sends the given configuration proposal
// to the underlying raft.
func TestNodeProposeConfig(t *testing.T) {
Expand Down
Loading
Loading