Skip to content

Commit

Permalink
raft: simplify auto-leave joint config of entry application logic
Browse files Browse the repository at this point in the history
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 etcd-io#14538.
- use `confChangeToMsg` to generate message, to centralize the creation
  of all `MsgProp{EntryConfChange}` messages.
  • Loading branch information
nvanbenschoten committed Oct 3, 2022
1 parent a932fb5 commit 645bfdd
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 13 deletions.
18 changes: 6 additions & 12 deletions raft/raft.go
Original file line number Diff line number Diff line change
Expand Up @@ -548,26 +548,20 @@ 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,
}
// 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")
}
r.bcastAppend()
r.pendingConfIndex = r.raftLog.lastIndex()
r.logger.Infof("initiating automatic transition out of joint configuration %s", r.prs.Config)
m, err := confChangeToMsg(nil)
if err != nil {
panic(err)
}
_ = r.Step(m)
}
}

Expand Down
8 changes: 7 additions & 1 deletion raft/raftpb/confchange.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 645bfdd

Please sign in to comment.