kvserver: mv entries cache update out of logstore

The current rough split between LogStore and replicaLogStorage is that
the latter is responsible for the in-memory state and keeping it
consistent with storage, and the former is just the storage part. This
commit moves the code in that direction by placing the raft entries
cache update in replicaLogStorage.

Epic: none
Release note: none

Author: pav-kv

pkg/kv/kvserver/BUILD.bazel

@@ -456,7 +456,6 @@ go_test(
         "//pkg/kv/kvserver/protectedts/ptpb",
         "//pkg/kv/kvserver/protectedts/ptstorage",
         "//pkg/kv/kvserver/protectedts/ptutil",
-        "//pkg/kv/kvserver/raftentry",
         "//pkg/kv/kvserver/raftlog",
         "//pkg/kv/kvserver/rafttrace",
         "//pkg/kv/kvserver/raftutil",

pkg/kv/kvserver/client_manual_proposal_test.go

@@ -19,7 +19,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/batcheval"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverpb"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/logstore"
-	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/raftentry"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/raftlog"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/stateloader"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/uncertainty"
@@ -223,7 +222,6 @@ LIMIT
 			Sideload:    nil,
 			StateLoader: rsl,
 			SyncWaiter:  swl,
-			EntryCache:  raftentry.NewCache(1024),
 			Settings:    st,
 		}
 

pkg/kv/kvserver/logstore/logstore.go

@@ -132,7 +132,6 @@ type LogStore struct {
 	Sideload    SideloadStorage
 	StateLoader StateLoader // used only for writes under raftMu
 	SyncWaiter  *SyncWaiterLoop
-	EntryCache  *raftentry.Cache
 	Settings    *cluster.Settings
 
 	DisableSyncLogWriteToss bool // for testing only
@@ -324,19 +323,6 @@ func (s *LogStore) storeEntriesAndCommitBatch(
 		}
 	}
 
-	// Update raft log entry cache. We clear any older, uncommitted log entries
-	// and cache the latest ones.
-	//
-	// In the blocking log sync case, these entries are already durable. In the
-	// non-blocking case, these entries have been written to the pebble engine (so
-	// reads of the engine will see them), but they are not yet be durable. This
-	// means that the entry cache can lead the durable log. This is allowed by
-	// etcd/raft, which maintains its own tracking of entry durability by
-	// splitting its log into an unstable portion for entries that are not known
-	// to be durable and a stable portion for entries that are known to be
-	// durable.
-	s.EntryCache.Add(s.RangeID, m.Entries, true /* truncate */)
-
 	return state, nil
 }
 
@@ -607,7 +593,7 @@ func LoadEntries(
 	ctx context.Context,
 	eng storage.Engine,
 	rangeID roachpb.RangeID,
-	eCache *raftentry.Cache,
+	eCache *raftentry.Cache, // TODO(#145562): this should be the caller's concern
 	sideloaded SideloadStorage,
 	lo, hi kvpb.RaftIndex,
 	maxBytes uint64,

pkg/kv/kvserver/logstore/logstore_bench_test.go

@@ -11,7 +11,6 @@ import (
 	"math/rand"
 	"testing"
 
-	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/raftentry"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/raftlog"
 	"github.com/cockroachdb/cockroach/pkg/raft"
 	"github.com/cockroachdb/cockroach/pkg/raft/raftpb"
@@ -48,8 +47,6 @@ func BenchmarkLogStore_StoreEntries(b *testing.B) {
 
 func runBenchmarkLogStore_StoreEntries(b *testing.B, bytes int64) {
 	ctx := context.Background()
-	const tenMB = 10 * 1 << 20
-	ec := raftentry.NewCache(tenMB)
 	const rangeID = 1
 	eng := storage.NewDefaultInMemForTesting()
 	defer eng.Close()
@@ -59,7 +56,6 @@ func runBenchmarkLogStore_StoreEntries(b *testing.B, bytes int64) {
 		RangeID:     rangeID,
 		Engine:      eng,
 		StateLoader: NewStateLoader(rangeID),
-		EntryCache:  ec,
 		Settings:    st,
 	}
 

pkg/kv/kvserver/replica_init.go

@@ -222,7 +222,6 @@ func newUninitializedReplicaWithoutRaftGroup(
 		// NOTE: use the same SyncWaiter loop for all raft log writes performed by a
 		// given range ID, to ensure that callbacks are processed in order.
 		SyncWaiter: store.syncWaiters[int(rangeID)%len(store.syncWaiters)],
-		EntryCache: store.raftEntryCache,
 		Settings:   store.cfg.Settings,
 		DisableSyncLogWriteToss: buildutil.CrdbTestBuild &&
 			store.TestingKnobs().DisableSyncLogWriteToss,

pkg/kv/kvserver/replica_raftlog_writes.go

@@ -58,7 +58,28 @@ func (r *replicaLogStorage) appendRaftMuLocked(
 ) (logstore.RaftState, error) {
 	state := r.stateRaftMuLocked()
 	cb := (*replicaSyncCallback)(r)
-	return r.raftMu.logStorage.StoreEntries(ctx, state, app, cb, stats)
+	state, err := r.raftMu.logStorage.StoreEntries(ctx, state, app, cb, stats)
+	if err != nil {
+		return state, err
+	}
+	// Update raft log entry cache. We clear any older, uncommitted log entries
+	// and cache the latest ones.
+	//
+	// In the blocking log sync case, these entries are already durable. In the
+	// non-blocking case, these entries have been written to storage (so reads of
+	// the engine will see them), but they are not yet durable. This means that
+	// the entry cache can lead the durable log. This is allowed by raft, which
+	// maintains its own tracking of entry durability by splitting its log into an
+	// unstable portion for entries that are not known to be durable and a stable
+	// portion for entries that are known to be durable.
+	//
+	// TODO(pav-kv): for safety, decompose this update into two steps: before
+	// writing to storage, truncate the suffix of the cache if entries are
+	// overwritten; after the write, append new entries to the cache. Currently,
+	// the cache can contain stale entries while storage is already updated, and
+	// the only hope is that nobody tries to read it under Replica.mu only.
+	r.store.raftEntryCache.Add(r.RangeID, app.Entries, true /* truncate */)
+	return state, nil
 }
 
 // updateStateRaftMuLockedMuLocked updates the in-memory reflection of the raft