[dnm] kvserver: use SingleDelete for raft log compaction

[pav-kv]

Epic: none
Release note: none

[blathers-crl]

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

[cockroach-teamcity]

This change is 

[sumeerbhola]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @pav-kv)

---

pkg/kv/kvserver/replica_raft.go line 2986 at r1 (raw file):

		// avoid allocating when constructing Raft log keys (16 bytes).
		prefix := prefixBuf.RaftLogPrefix()
		for idx := currentTruncatedState.Index + 1; idx <= suggestedTruncatedState.Index; idx++ {

What code paths are used to delete the raft log when

• a range snapshot is received
• the range is dropped
  Is my understanding correct in that in both the above cases all the raft log entries are wiped out.

I think we could more safely make this change if we centralized the logic that modifies the raft log entries into one package. Maybe logstore?

logstore.logAppend will need some attention. storage.MVCCPut when overwriting an existing Raft entry will eventually do storage.Writer.PutUnversioned, which will layer a Pebble SET on top of an existing SET, which cannot be later deleted using a SINGLEDEL.

[pav-kv]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @sumeerbhola)

---

pkg/kv/kvserver/replica_raft.go line 2986 at r1 (raw file):

Previously, sumeerbhola wrote…

What code paths are used to delete the raft log when

• a range snapshot is received
• the range is dropped
  Is my understanding correct in that in both the above cases all the raft log entries are wiped out.

I think we could more safely make this change if we centralized the logic that modifies the raft log entries into one package. Maybe logstore?

logstore.logAppend will need some attention. storage.MVCCPut when overwriting an existing Raft entry will eventually do storage.Writer.PutUnversioned, which will layer a Pebble SET on top of an existing SET, which cannot be later deleted using a SINGLEDEL.

• snapshots
• replica destruction

Also, there is a write in logstore package that you mention which I glossed over in this prototype because it's rare. When overwriting an entry (we always know when we're about to), we could SINGLEDEL+SET instead of just SET.

Agree that logstore/etc needs to be cleaned up as part of this work (and also relatedly to sep-raft-log). Right now, it's scattered across the codebase: logstore, replica_raftlog.go, and a few places writing raft state to storage directly.