keys: resolve subtle non-bug by exporting RaftLogKeyFromPrefix

pkg/keys/keys.go

@@ -331,6 +331,19 @@ func RaftLogKey(rangeID roachpb.RangeID, logIndex uint64) roachpb.Key {
 	return MakeRangeIDPrefixBuf(rangeID).RaftLogKey(logIndex)
 }
 
+// RaftLogKeyFromPrefix returns a system-local key for a Raft log entry, using
+// the provided Raft log prefix.
+func RaftLogKeyFromPrefix(raftLogPrefix []byte, logIndex uint64) roachpb.Key {
+	return encoding.EncodeUint64Ascending(raftLogPrefix, logIndex)
+}
+
+// DecodeRaftLogKeyFromSuffix parses the suffix of a system-local key for a Raft
+// log entry and returns the entry's log index.
+func DecodeRaftLogKeyFromSuffix(raftLogSuffix []byte) (uint64, error) {
+	_, logIndex, err := encoding.DecodeUint64Ascending(raftLogSuffix)
+	return logIndex, err
+}
+
 // RaftReplicaIDKey returns a system-local key for a RaftReplicaID.
 func RaftReplicaIDKey(rangeID roachpb.RangeID) roachpb.Key {
 	return MakeRangeIDPrefixBuf(rangeID).RaftReplicaIDKey()
@@ -447,7 +460,7 @@ func LockTableSingleKey(key roachpb.Key, buf []byte) (roachpb.Key, []byte) {
 }
 
 // DecodeLockTableSingleKey decodes the single-key lock table key to return the key
-// that was locked..
+// that was locked.
 func DecodeLockTableSingleKey(key roachpb.Key) (lockedKey roachpb.Key, err error) {
 	if !bytes.HasPrefix(key, LocalRangeLockTablePrefix) {
 		return nil, errors.Errorf("key %q does not have %q prefix",
@@ -1029,7 +1042,7 @@ func (b RangeIDPrefixBuf) RaftLogPrefix() roachpb.Key {
 
 // RaftLogKey returns a system-local key for a Raft log entry.
 func (b RangeIDPrefixBuf) RaftLogKey(logIndex uint64) roachpb.Key {
-	return encoding.EncodeUint64Ascending(b.RaftLogPrefix(), logIndex)
+	return RaftLogKeyFromPrefix(b.RaftLogPrefix(), logIndex)
 }
 
 // RaftReplicaIDKey returns a system-local key for a RaftReplicaID.

pkg/kv/kvserver/replica_raft.go

@@ -2110,8 +2110,9 @@ func handleTruncatedStateBelowRaftPreApply(
 	} else {
 		// NB: RangeIDPrefixBufs have sufficient capacity (32 bytes) to
 		// avoid allocating when constructing Raft log keys (16 bytes).
+		prefix := prefixBuf.RaftLogPrefix()
 		for idx := currentTruncatedState.Index + 1; idx <= suggestedTruncatedState.Index; idx++ {
-			if err := readWriter.ClearUnversioned(prefixBuf.RaftLogKey(idx)); err != nil {
+			if err := readWriter.ClearUnversioned(keys.RaftLogKeyFromPrefix(prefix, idx)); err != nil {
 				return false, errors.Wrapf(err, "unable to clear truncated Raft entries for %+v at index %d",
 					suggestedTruncatedState, idx)
 			}

pkg/kv/kvserver/replica_raftstorage.go

@@ -649,11 +649,12 @@ func (r *Replica) append(
 	if len(entries) == 0 {
 		return prevLastIndex, prevLastTerm, prevRaftLogSize, nil
 	}
+	prefix := r.raftMu.stateLoader.RaftLogPrefix()
 	var diff enginepb.MVCCStats
 	var value roachpb.Value
 	for i := range entries {
 		ent := &entries[i]
-		key := r.raftMu.stateLoader.RaftLogKey(ent.Index)
+		key := keys.RaftLogKeyFromPrefix(prefix, ent.Index)
 
 		if err := value.SetProto(ent); err != nil {
 			return 0, 0, 0, err
@@ -689,7 +690,7 @@ func (r *Replica) append(
 		for i := lastIndex + 1; i <= prevLastIndex; i++ {
 			// Note that the caller is in charge of deleting any sideloaded payloads
 			// (which they must only do *after* the batch has committed).
-			err := storage.MVCCDelete(ctx, eng, &diff, r.raftMu.stateLoader.RaftLogKey(i),
+			err := storage.MVCCDelete(ctx, eng, &diff, keys.RaftLogKeyFromPrefix(prefix, i),
 				hlc.Timestamp{}, hlc.ClockTimestamp{}, nil)
 			if err != nil {
 				return 0, 0, 0, err

pkg/kv/kvserver/stateloader/BUILD.bazel

@@ -15,7 +15,6 @@ go_library(
         "//pkg/roachpb",
         "//pkg/storage",
         "//pkg/storage/enginepb",
-        "//pkg/util/encoding",
         "//pkg/util/hlc",
         "//pkg/util/log",
         "//pkg/util/protoutil",

pkg/kv/kvserver/stateloader/stateloader.go

@@ -19,7 +19,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/storage"
 	"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
-	"github.com/cockroachdb/cockroach/pkg/util/encoding"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/protoutil"
@@ -45,7 +44,7 @@ type StateLoader struct {
 	keys.RangeIDPrefixBuf
 }
 
-// Make creates a a StateLoader.
+// Make creates a StateLoader.
 func Make(rangeID roachpb.RangeID) StateLoader {
 	rsl := StateLoader{
 		RangeIDPrefixBuf: keys.MakeRangeIDPrefixBuf(rangeID),
@@ -300,13 +299,17 @@ func (rsl StateLoader) LoadLastIndex(ctx context.Context, reader storage.Reader)
 	defer iter.Close()
 
 	var lastIndex uint64
-	iter.SeekLT(storage.MakeMVCCMetadataKey(rsl.RaftLogKey(math.MaxUint64)))
+	iter.SeekLT(storage.MakeMVCCMetadataKey(keys.RaftLogKeyFromPrefix(prefix, math.MaxUint64)))
 	if ok, _ := iter.Valid(); ok {
-		key := iter.Key()
+		key := iter.UnsafeKey().Key
+		if len(key) < len(prefix) {
+			log.Fatalf(ctx, "unable to decode Raft log index key: len(%s) < len(%s)", key.String(), prefix.String())
+		}
+		suffix := key[len(prefix):]
 		var err error
-		_, lastIndex, err = encoding.DecodeUint64Ascending(key.Key[len(prefix):])
+		lastIndex, err = keys.DecodeRaftLogKeyFromSuffix(suffix)
 		if err != nil {
-			log.Fatalf(ctx, "unable to decode Raft log index key: %s", key)
+			log.Fatalf(ctx, "unable to decode Raft log index key: %s; %v", key.String(), err)
 		}
 	}