Merge #120613 #121956 #122011

120613: raft: remove unused read-only requests r=nvanbenschoten a=pav-kv

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

121956: sql: add accounting for entries in Txn Fingerprint ID cache r=yuzefovich a=yuzefovich

This commit fixes a bug that could previously result in the memory accounting leak that was exposed by 88ebd70. Namely, the problem is that we previously unconditionally grew the memory account in `Add` even though if the ID is already present in the cache, it wouldn't be inserted again. As a result, we'd only shrink the account once but might have grown it any number of times for a particular ID. Now we check whether the ID is present in the cache and only grow the account if not.

Epic: None

Release note: None

122011: backupccl: skip BackuprestoreJobDescription under stress race r=kev-cao a=msbutler

Informs #121927

Release note: none

Co-authored-by: Pavel Kalinnikov <pavel@cockroachlabs.com>
Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
Co-authored-by: Michael Butler <butler@cockroachlabs.com>

pkg/ccl/backupccl/backup_test.go

@@ -733,6 +733,8 @@ func TestBackupAndRestoreJobDescription(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
+	skip.UnderStressRace(t, "this test is heavyweight and is not expected to reveal any direct bugs under stress race")
+
 	const numAccounts = 1
 	_, sqlDB, tmpDir, cleanupFn := backupRestoreTestSetup(t, multiNode, numAccounts, InitManualReplication)
 	defer cleanupFn()

pkg/kv/kvserver/replica_raft.go

@@ -1208,12 +1208,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.
 	//
@@ -1225,9 +1219,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,
 	}
 }
 

pkg/raft/BUILD.bazel

@@ -11,7 +11,6 @@ go_library(
         "node.go",
         "raft.go",
         "rawnode.go",
-        "read_only.go",
         "status.go",
         "storage.go",
         "types.go",

pkg/raft/node.go

@@ -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.
 	//
@@ -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.
@@ -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}}})
-}

pkg/raft/node_test.go

@@ -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) {