raft: clean-up log conflict search

[pav-kv]

Use the new entryID and logSlice types. Move the preceding entry check into the conflict search method rather than do it outside. Add a bunch of TODOs for optimization: most log append requests can skip the term check scanning or do it more efficiently.

Epic: CRDB-37516
Release note: none

[cockroach-teamcity]

This change is 

[miraradeva]

Just a couple of questions, for my own understanding of Raft.

Reviewed 1 of 1 files at r1, 2 of 2 files at r2.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @arulajmani and @pav-kv)

---

pkg/raft/log.go line 165 at r2 (raw file):

	// Check the term match at min(s.lastIndex(), l.lastIndex()) entry, and fall
	// back to conflict search only if it mismatches.
	// TODO(pav-kv): also, there should be no mismatch if s.term == l.accTerm, so

What's l.accTerm?

---

pkg/raft/log.go line 170 at r2 (raw file):

	// TODO(pav-kv): every matchTerm call in the linear scan below can fall back
	// to fetching an entry from storage. This is inefficient, we can improve it.
	// Logs that don't match at one index, don't match at all indices above. So we

Is this always true? Could two logs have disjoint matching parts? E.g. if the second entry with term 1 below is missing from the second log:
log1: (index: 1, term: 1), (index: 2, term: 1), (index: 3, term: 2), (index: 4, term: 3) ...
log2: (index: 1, term: 1), (index: 2, term: 2), (index: 3, term: 3), (index: 4, term: 3) ...

Or will the missing entry correspond to a missing index as well? So it will look like this:
log1: (index: 1, term: 1), (index: 2, term: 1), (index: 3, term: 2), (index: 4, term: 3) ...
log2: (index: 1, term: 1), (index: 3, term: 2), (index: 4, term: 3), (index: 5, term: 3) ...

It seems like it's the latter, based on the Log Matching property of raft you mention above.

[pav-kv]

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @arulajmani and @miraradeva)

---

pkg/raft/log.go line 165 at r2 (raw file):

Previously, miraradeva (Mira Radeva) wrote…

What's l.accTerm?

It is the term of the leader whose append we accepted last:

cockroach/pkg/raft/raft.go

Lines 348 to 363 in 5769b24

	// accTerm is the term of the leader whose append was accepted into the log
	// last. Note that a rejected append does not update accTerm, by definition.
	//
	// Invariant: the log is a prefix of the accTerm's leader log
	// Invariant: raftLog.lastEntryID().term <= accTerm <= Term
	//
	// In steady state, accTerm == Term. When someone campaigns, Term briefly
	// overtakes the accTerm. However, accTerm catches up as soon as we accept a
	// log append from the new leader.
	//
	// NB: the log can be partially or fully compacted. When we say "log" above,
	// we logically include all the entries that were the pre-image of a snapshot,
	// as well as the entries that are still physically in the log.
	//
	// TODO(pav-kv): move accTerm to raftLog.
	accTerm uint64

By raft invariants, we have a guarantee that our log is a prefix of the accTerm leader's log. So, if the accTerm leader sends us log appends, we may skip all the matchTerm checks (they will all succeed).

---

pkg/raft/log.go line 170 at r2 (raw file):

Previously, miraradeva (Mira Radeva) wrote…

Is this always true? Could two logs have disjoint matching parts? E.g. if the second entry with term 1 below is missing from the second log:
log1: (index: 1, term: 1), (index: 2, term: 1), (index: 3, term: 2), (index: 4, term: 3) ...
log2: (index: 1, term: 1), (index: 2, term: 2), (index: 3, term: 3), (index: 4, term: 3) ...

Or will the missing entry correspond to a missing index as well? So it will look like this:
log1: (index: 1, term: 1), (index: 2, term: 1), (index: 3, term: 2), (index: 4, term: 3) ...
log2: (index: 1, term: 1), (index: 3, term: 2), (index: 4, term: 3), (index: 5, term: 3) ...

It seems like it's the latter, based on the Log Matching property of raft you mention above.

Yes, it is the latter. Both of the following are true:

• if two logs have the same entry at index, then all the entries at lower indices match too
• if two logs mismatch at some index, they also mismatch at all higher indices

This is true because all the entries in raft form a tree. See "Background" section in etcd-io/raft#144 for more details.

[pav-kv]

bors r=miraradeva

[craig]

Build succeeded:

• macos_arm64_build
• check_generated_code
• linux_amd64_fips_build
• local_roachtest_fips
• docker_image_amd64
• unit_tests
• linux_amd64_build
• acceptance
• macos_amd64_build
• examples_orms
• lint
• linux_arm64_build
• local_roachtest
• windows_build