logstore: rm MaybeInlineSideloadedRaftCommand alloc

Epic: none
Release note: none

Author: pav-kv

pkg/kv/kvserver/logstore/logstore.go

@@ -740,20 +740,14 @@ func LoadEntries(
 		}
 		expectedIndex++
 
-		typ, _, err := raftlog.EncodingOf(ent)
-		if err != nil {
+		if typ, _, err := raftlog.EncodingOf(ent); err != nil {
 			return err
-		}
-		if typ.IsSideloaded() {
-			newEnt, err := MaybeInlineSideloadedRaftCommand(
+		} else if typ.IsSideloaded() {
+			if ent, err = MaybeInlineSideloadedRaftCommand(
 				ctx, rangeID, ent, sideloaded, eCache,
-			)
-			if err != nil {
+			); err != nil {
 				return err
 			}
-			if newEnt != nil {
-				ent = *newEnt
-			}
 		}
 
 		if sh.add(uint64(ent.Size())) {

pkg/kv/kvserver/logstore/sideload.go

@@ -149,8 +149,8 @@ func MaybeSideloadEntries(
 
 // MaybeInlineSideloadedRaftCommand takes an entry and inspects it. If its
 // command encoding version indicates a sideloaded entry, it uses the entryCache
-// or SideloadStorage to inline the payload, returning a new entry (which must
-// be treated as immutable by the caller) or nil (if inlining does not apply)
+// or SideloadStorage to inline the payload, and returns a new entry (which must
+// be treated as immutable by the caller).
 //
 // If a payload is missing, returns an error whose Cause() is
 // errSideloadedFileNotFound.
@@ -160,31 +160,24 @@ func MaybeInlineSideloadedRaftCommand(
 	ent raftpb.Entry,
 	sideloaded SideloadStorage,
 	entryCache *raftentry.Cache,
-) (*raftpb.Entry, error) {
+) (raftpb.Entry, error) {
 	typ, pri, err := raftlog.EncodingOf(ent)
-	if err != nil {
-		return nil, err
-	}
-	if !typ.IsSideloaded() {
-		return nil, nil
+	if err != nil || !typ.IsSideloaded() {
+		return ent, err
 	}
 	log.Event(ctx, "inlining sideloaded SSTable")
 	// We could unmarshal this yet again, but if it's committed we are very likely
 	// to have appended it recently, in which case we can save work.
 	if entry, hit := entryCache.Get(rangeID, kvpb.RaftIndex(ent.Index)); hit {
 		log.Event(ctx, "using cache hit")
-		return &entry, nil
+		return entry, nil
 	}
 
-	// Make a shallow copy.
-	entCpy := ent
-	ent = entCpy
-
 	log.Event(ctx, "inlined entry not cached")
 	// (Bad) luck, for whatever reason the inlined proposal isn't in the cache.
 	e, err := raftlog.NewEntry(ent)
 	if err != nil {
-		return nil, err
+		return ent, err
 	}
 
 	if len(e.Cmd.ReplicatedEvalResult.AddSSTable.Data) > 0 {
@@ -196,12 +189,12 @@ func MaybeInlineSideloadedRaftCommand(
 		// be as a result of log entries that are very old, written
 		// when sending the log with snapshots was still possible).
 		log.Event(ctx, "entry already inlined")
-		return &ent, nil
+		return ent, nil
 	}
 
 	sideloadedData, err := sideloaded.Get(ctx, kvpb.RaftIndex(ent.Index), kvpb.RaftTerm(ent.Term))
 	if err != nil {
-		return nil, errors.Wrap(err, "loading sideloaded data")
+		return ent, errors.Wrap(err, "loading sideloaded data")
 	}
 	e.Cmd.ReplicatedEvalResult.AddSSTable.Data = sideloadedData
 	// TODO(tbg): there should be a helper that properly encodes a command, given
@@ -211,11 +204,11 @@ func MaybeInlineSideloadedRaftCommand(
 		raftlog.EncodeRaftCommandPrefix(data[:raftlog.RaftCommandPrefixLen], typ, e.ID, pri)
 		_, err := protoutil.MarshalToSizedBuffer(&e.Cmd, data[raftlog.RaftCommandPrefixLen:])
 		if err != nil {
-			return nil, err
+			return ent, err
 		}
 		ent.Data = data
 	}
-	return &ent, nil
+	return ent, nil
 }
 
 // AssertSideloadedRaftCommandInlined asserts that if the provided entry is a

pkg/kv/kvserver/logstore/sideload_test.go

@@ -451,11 +451,7 @@ func TestRaftSSTableSideloadingInline(t *testing.T) {
 		} else {
 			mustEntryEq(t, thinCopy, test.thin)
 		}
-
-		if newEnt == nil {
-			newEnt = &thinCopy
-		}
-		mustEntryEq(t, *newEnt, test.fat)
+		mustEntryEq(t, newEnt, test.fat)
 
 		if dump := getRecAndFinish().String(); test.expTrace != "" {
 			if ok, err := regexp.MatchString(test.expTrace, dump); err != nil {