From c50e728518ef3141d1ffe58db9a77ee8928e5d05 Mon Sep 17 00:00:00 2001
From: Nathan VanBenschoten <nvanbenschoten@gmail.com>
Date: Mon, 3 Oct 2022 01:56:07 -0400
Subject: [PATCH] raft: simplify auto-leave joint config on entry application
 logic

This commit simplifies the logic added in 37c7e4d to auto-leave joint
configurations. It does so by making the following adjustments to the
code:

- remove the `oldApplied <= r.pendingConfIndex` condition. This does
  not seem necessary. When a node first attempts to auto-leave a joint
  config, it will bump `r.pendingConfIndex` when proposing. In cases
  where `oldApplied >= r.pendingConfIndex`, the proposal must have
  already been applied. Reviewers should double check this.
- use raft.Step instead of custom proposal code. This code was already
  present in stepLeader, so there was no reason to duplicate it. This
  would have avoided bugs like the one we fixed in #14538.
- use `confChangeToMsg` to generate message, to centralize the creation
  of all `MsgProp{EntryConfChange}` messages.

Signed-off-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
---
 raft/raft.go              | 25 ++++++++++++++-----------
 raft/raftpb/confchange.go |  8 +++++++-
 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/raft/raft.go b/raft/raft.go
index 66048c789..174611821 100644
--- a/raft/raft.go
+++ b/raft/raft.go
@@ -548,26 +548,29 @@ func (r *raft) advance(rd Ready) {
 	// new Commit index, this does not mean that we're also applying
 	// all of the new entries due to commit pagination by size.
 	if newApplied := rd.appliedCursor(); newApplied > 0 {
-		oldApplied := r.raftLog.applied
 		r.raftLog.appliedTo(newApplied)
 
-		if r.prs.Config.AutoLeave && oldApplied <= r.pendingConfIndex && newApplied >= r.pendingConfIndex && r.state == StateLeader {
+		if r.prs.Config.AutoLeave && newApplied >= r.pendingConfIndex && r.state == StateLeader {
 			// If the current (and most recent, at least for this leader's term)
 			// configuration should be auto-left, initiate that now. We use a
 			// nil Data which unmarshals into an empty ConfChangeV2 and has the
 			// benefit that appendEntry can never refuse it based on its size
 			// (which registers as zero).
-			ent := pb.Entry{
-				Type: pb.EntryConfChangeV2,
-				Data: nil,
+			m, err := confChangeToMsg(nil)
+			if err != nil {
+				panic(err)
 			}
-			// There's no way in which this proposal should be able to be rejected.
-			if !r.appendEntry(ent) {
-				panic("refused un-refusable auto-leaving ConfChangeV2")
+			// NB: this proposal can't be dropped due to size, but can be
+			// dropped if a leadership transfer is in progress. We'll keep
+			// checking this condition on each applied entry, so either the
+			// leadership transfer will succeed and the new leader will leave
+			// the joint configuration, or the leadership transfer will fail,
+			// and we will propose the config change on the next advance.
+			if err := r.Step(m); err != nil {
+				r.logger.Debugf("not initiating automatic transition out of joint configuration %s: %v", r.prs.Config, err)
+			} else {
+				r.logger.Infof("initiating automatic transition out of joint configuration %s", r.prs.Config)
 			}
-			r.bcastAppend()
-			r.pendingConfIndex = r.raftLog.lastIndex()
-			r.logger.Infof("initiating automatic transition out of joint configuration %s", r.prs.Config)
 		}
 	}
 
diff --git a/raft/raftpb/confchange.go b/raft/raftpb/confchange.go
index 47fae65df..a3ddff62f 100644
--- a/raft/raftpb/confchange.go
+++ b/raft/raftpb/confchange.go
@@ -35,7 +35,13 @@ func MarshalConfChange(c ConfChangeI) (EntryType, []byte, error) {
 	var typ EntryType
 	var ccdata []byte
 	var err error
-	if ccv1, ok := c.AsV1(); ok {
+	if c == nil {
+		// A nil data unmarshals into an empty ConfChangeV2 and has the benefit
+		// that appendEntry can never refuse it based on its size (which
+		// registers as zero).
+		typ = EntryConfChangeV2
+		ccdata = nil
+	} else if ccv1, ok := c.AsV1(); ok {
 		typ = EntryConfChange
 		ccdata, err = ccv1.Marshal()
 	} else {