raft: advance commit index safely

[lyang24]

This change makes the commit index advancement in handleHeartbeat safe. Previously, a follower would attempt to update the commit index to whichever was sent in the MsgHeartbeat message. Out-of-bound indices would crash the node.

It is always safe to advance a commit index if the follower's log is "in sync" with the leader, i.e. when its log is guaranteed to be a prefix of the leader's log. This is always true if the term of last entry in the log matches the leader's term, otherwise this guarantee is established when the first MsgApp append message from the leader succeeds.

At the moment, the leader will never send a commit index that exceeds the follower's log size. However, this may change in future. This change is a defence-in-depth.

The newly added accTerm field will be used for other safety checks in the future, for example to establish that overriding a suffix of entries in raftLog is safe.

Part of #122100

[blathers-crl[bot]]

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

I was unable to automatically find a reviewer. You can try CCing one of the following members:

• A person you worked with closely on this PR.
• The person who created the ticket, or a CRDB organization member involved with the ticket (author, commenter, etc.).
• Join our community slack channel and ask on #contributors.
• Try find someone else from here.

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

[cockroach-teamcity]

This change is 

[blathers-crl[bot]]

Thank you for updating your pull request.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

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

[pav-kv]

LGTM, but let's have a second review. @nvanbenschoten Could you PTAL?

[lyang24]

@lyang24 the blocking PR has merged. Please address Nathan's comments, and it should be safe to merge this one.

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

pkg/raft/raft.go line 1729 at r4 (raw file):

Previously, pav-kv (Pavel Kalinnikov) wrote… The PR has merged. It should be safe to make the change now.

Hi I have rebased to master will merge when tests passes and will work on the follow up prs.

[lyang24]

bors r+

[craig[bot]]

:clock1: Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

[pav-kv]

bors cancel

[pav-kv]

@lyang24 there are some outstanding comments from @nvanbenschoten, could you please address them first?

@nvanbenschoten Do you want to take a final pass / LGTM?

[lyang24]

bors r+

[craig[bot]]

Build succeeded:

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