Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

server: transfer leadership in case of error #12293

Merged
merged 5 commits into from
Mar 17, 2022
Merged

server: transfer leadership in case of error #12293

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cb4ae1f
server: transfer leadership in case of error
lgfa29 Mar 14, 2022
b15b7be
changelog: add entry for #12293
lgfa29 Mar 14, 2022
2dc1ff1
return early when leadership transfer fail due to protocol version
lgfa29 Mar 16, 2022
4ffa808
docs: add upgrade note about raft leadership transfer
lgfa29 Mar 16, 2022
772abb6
docs: rewrite raft leadership transfer upgrade note
lgfa29 Mar 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/12293.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
server: Transfer Raft leadership in case the Nomad server fails to establish leadership
```
56 changes: 54 additions & 2 deletions nomad/leader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,27 @@ func (s *Server) monitorLeadership() {
}
}

func (s *Server) leadershipTransfer() error {
retryCount := 3
for i := 0; i < retryCount; i++ {
future := s.raft.LeadershipTransfer()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LeadershipTransfer will fail if the Raft version is < 3, which would cause the cluster to get stuck in a leaderless infinite loop since we always retry if leadershipTransfer returns an error.

    2022-03-14T17:13:31.725-0400 [ERROR] nomad: failed to establish leadership: error="i can't be a leader"
    2022-03-14T17:13:31.725-0400 [ERROR] nomad: failed to transfer leadership attempt, will retry: attempt=0 retry_limit=3 error="operation not supported with current protocol version"
    2022-03-14T17:13:31.725-0400 [ERROR] nomad: failed to transfer leadership attempt, will retry: attempt=1 retry_limit=3 error="operation not supported with current protocol version"
    2022-03-14T17:13:31.725-0400 [ERROR] nomad: failed to transfer leadership attempt, will retry: attempt=2 retry_limit=3 error="operation not supported with current protocol version"
    2022-03-14T17:13:31.725-0400 [ERROR] nomad: failed to transfer leadership: error="failed to transfer leadership in 3 attempts"
    2022-03-14T17:13:36.822-0400 [ERROR] nomad: failed to establish leadership: error="i can't be a leader"
    2022-03-14T17:13:36.822-0400 [ERROR] nomad: failed to transfer leadership attempt, will retry: attempt=0 retry_limit=3 error="operation not supported with current protocol version"
    2022-03-14T17:13:36.822-0400 [ERROR] nomad: failed to transfer leadership attempt, will retry: attempt=1 retry_limit=3 error="operation not supported with current protocol version"
    2022-03-14T17:13:36.822-0400 [ERROR] nomad: failed to transfer leadership attempt, will retry: attempt=2 retry_limit=3 error="operation not supported with current protocol version"
    2022-03-14T17:13:36.822-0400 [ERROR] nomad: failed to transfer leadership: error="failed to transfer leadership in 3 attempts"
    2022-03-14T17:13:41.908-0400 [ERROR] nomad: failed to establish leadership: error="i can't be a leader"
    2022-03-14T17:13:41.908-0400 [ERROR] nomad: failed to transfer leadership attempt, will retry: attempt=0 retry_limit=3 error="operation not supported with current protocol version"
    2022-03-14T17:13:41.908-0400 [ERROR] nomad: failed to transfer leadership attempt, will retry: attempt=1 retry_limit=3 error="operation not supported with current protocol version"
    2022-03-14T17:13:41.908-0400 [ERROR] nomad: failed to transfer leadership attempt, will retry: attempt=2 retry_limit=3 error="operation not supported with current protocol version"
    2022-03-14T17:13:41.908-0400 [ERROR] nomad: failed to transfer leadership: error="failed to transfer leadership in 3 attempts"

Should we keep the previous (erroneous) behaviour if Raft is not v3? Or is there something else we can do?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we keep the previous (erroneous) behaviour if Raft is not v3?

That seems fine to me. Let's add a note in the upgrade guide about this behavior change. If nothing else it's a nice concrete feature to encourage upgrading.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, I will work on that, thanks!

Copy link
Contributor Author

@lgfa29 lgfa29 Mar 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@schmichael After staring waaay too long at this, I realized that the previous behaviour is actually for the leader to get stuck, which is also the point of this fix, so there isn't much we can do about it.

In 2dc1ff1 I changed the logic a little to return early and with a more meaningful message if the problem is unsupported Raft version and in 772abb6 I added the upgrade guide notice.

if err := future.Error(); err != nil {
s.logger.Error("failed to transfer leadership attempt, will retry",
"attempt", i,
"retry_limit", retryCount,
"error", err,
)
} else {
s.logger.Info("successfully transferred leadership",
"attempt", i,
"retry_limit", retryCount,
)
return nil
}
}
return fmt.Errorf("failed to transfer leadership in %d attempts", retryCount)
}

// leaderLoop runs as long as we are the leader to run various
// maintenance activities
func (s *Server) leaderLoop(stopCh chan struct{}) {
Expand Down Expand Up @@ -151,7 +172,15 @@ RECONCILE:
s.logger.Error("failed to revoke leadership", "error", err)
}

goto WAIT
// Attempt to transfer leadership. If successful, leave the
// leaderLoop since this node is no longer the leader. Otherwise
// try to establish leadership again after 5 seconds.
if err := s.leadershipTransfer(); err != nil {
s.logger.Error("failed to transfer leadership", "error", err)
interval = time.After(5 * time.Second)
goto WAIT
}
return
}

establishedLeader = true
Expand Down Expand Up @@ -182,10 +211,12 @@ RECONCILE:
}

WAIT:
// Wait until leadership is lost
// Wait until leadership is lost or periodically reconcile as long as we
// are the leader, or when Serf events arrive.
for {
select {
case <-stopCh:
// Lost leadership.
return
case <-s.shutdownCh:
return
Expand Down Expand Up @@ -213,6 +244,27 @@ WAIT:
s.revokeLeadership()
err := s.establishLeadership(stopCh)
errCh <- err

// In case establishLeadership fails, try to transfer leadership.
// At this point Raft thinks we are the leader, but Nomad did not
// complete the required steps to act as the leader.
if err != nil {
if err := s.leadershipTransfer(); err != nil {
// establishedLeader was true before, but it no longer is
// since we revoked leadership and leadershipTransfer also
// failed.
// Stay in the leaderLoop with establishedLeader set to
// false so we try to establish leadership again in the
// next loop.
establishedLeader = false
interval = time.After(5 * time.Second)
goto WAIT
}

// leadershipTransfer was successful and it is
// time to leave the leaderLoop.
return
}
}
}
}
Expand Down