storage: delete all "raft log as part of snapshots" code

[irfansharif]

Raft log entries are no longer sent along with snapshots as of v19.2
(more specifically, #35701), we can/should delete all code around the
sending/handling of log entries in snapshots before v20.1 is cut. This
simplifies a few snapshot related protos and naturally obviates the need
to handle sideloaded proposals within snapshots.

Release note: None

[cockroach-teamcity]

This change is 

[ajwerner]

yay deleting code

Reviewed 5 of 7 files at r1.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @ajwerner, @irfansharif, @nvanbenschoten, and @tbg)

---

pkg/storage/replica_raftstorage.go, line 443 at r1 (raw file):

	Iter *rditer.ReplicaDataIterator
	// The replica state within the snapshot.
	State storagepb.ReplicaState

This needs a newline or a gofmt.

---

pkg/storage/replica_raftstorage.go, line 1057 at r1 (raw file):

}

// extractRangeFromEntries returns a string representation of the range of

This is no longer used

---

pkg/storage/store_snapshot.go, line 762 at r1 (raw file):

}

type errMustRetrySnapshotDueToTruncation struct {

This is no longer used.

[nvanbenschoten]

this is another one I'd let @tbg sign off on as well before merging.

yay deleting code

keep going! What else can we delete?

Reviewed 5 of 7 files at r1.
Reviewable status: complete! 2 of 0 LGTMs obtained (waiting on @irfansharif and @tbg)

---

pkg/storage/replica_raftstorage.go, line 795 at r1 (raw file):

	}

	// Update Raft entries.

Could you add a comment here explaining what now happens with the recipient's Raft log?

---

pkg/storage/store_snapshot.go, line 288 at r1 (raw file):

			}

			expLen := inSnap.State.RaftAppliedIndex - inSnap.State.TruncatedState.Index

Is it worth asserting that this is always 0 so that we know we were never given any log entries from old versions of CRDB?

[irfansharif]

TFTRs, I'll wait for Tobi to chime in.

What else can we delete?

I'm only really familiar with the more recent deprecations, you folks might know better. But very naively grepping for 19.{1,2} in code comments (I'm very likely wrong here, didn't dig in deeper) I see:

1. ChangeReplicasTrigger has a few deprecated fields we should no longer be using, could strip out a bit. Also this and this
2. There's a bit of cleanup around unsplit command code
3. Minor cleanup around sorting behavior of InternalReplicas here
4. Some stats handling here
5. Leftover TODOs around intent handling here
6. Code around checking if replicated eval result side effects are trivial
7. Can probably remove this stats inconsistency panic hatch

Other than this, I can only think of stuff to clean up post #39182. Pre-emptive snapshots would be a nice one to strip out. RaftTruncatedStateLegacyKey is another, which IIUC would let us delete cmd_truncate_log.go + handleTruncatedStateBelowRaft in its entirety.

Reviewable status: complete! 2 of 0 LGTMs obtained (waiting on @ajwerner, @nvanbenschoten, and @tbg)

---

pkg/storage/replica_raftstorage.go, line 443 at r1 (raw file):

Previously, ajwerner wrote…

This needs a newline or a gofmt.

Ugh, for some reason my goland filewatchers keep resetting. I should figure out what's happening.

---

pkg/storage/replica_raftstorage.go, line 795 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

Could you add a comment here explaining what now happens with the recipient's Raft log?

Done.

---

pkg/storage/replica_raftstorage.go, line 1057 at r1 (raw file):

Previously, ajwerner wrote…

This is no longer used

Done (was leaning on CI lint checks).

---

pkg/storage/store_snapshot.go, line 288 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

Is it worth asserting that this is always 0 so that we know we were never given any log entries from old versions of CRDB?

Assert what to zero? Not sure I follow. (I have to clean up the code block below this comment, but stepping out for a bit.)

---

pkg/storage/store_snapshot.go, line 762 at r1 (raw file):

Previously, ajwerner wrote…

This is no longer used.

Done (was leaning on CI lint checks).

[irfansharif]

Reviewable status: complete! 2 of 0 LGTMs obtained (waiting on @ajwerner, @nvanbenschoten, and @tbg)

---

pkg/storage/store_snapshot.go, line 288 at r1 (raw file):

Previously, irfansharif (irfan sharif) wrote…

Assert what to zero? Not sure I follow. (I have to clean up the code block below this comment, but stepping out for a bit.)

Oh, reviewable was not rendering the diff for some reason prior. I saw an isolated comment in the middle of no where. Done.

[tbg]

Hmm, it's late here and maybe I'm asleep at the wheel, but ranges that still use the legacy replicated truncated state must include the raft log. I don't think it's safe to remove this until we have #42822 and have migrated across it?

[irfansharif]

You're right. Going to let this sit for now.

[irfansharif]

I don't think it's safe to remove this until we have #42822 and have migrated across it?

This was done in #58088, which means this PR will only be applicable in the v21.2 cycle, which is forever away. Closing it out.