Skip to content

elector: resign leadership w/ WithoutCancel ctx, not background #403

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

Merged
brandur merged 2 commits into from
Jun 26, 2024
Merged

elector: resign leadership w/ WithoutCancel ctx, not background #403

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
04a975d
elector: always use Logger.*Context() methods
bgentry Jun 26, 2024
569ed24
elector: resign leadership w/ WithoutCancel ctx, not background
bgentry Jun 26, 2024
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
21 changes: 11 additions & 10 deletions internal/leadership/elector.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ func (e *Elector) Start(ctx context.Context) error {
continue // lost leadership reelection; unusual but not a problem; don't log
}

e.Logger.Error(e.Name+": Error keeping leadership", "client_id", e.config.ClientID, "err", err)
e.Logger.ErrorContext(ctx, e.Name+": Error keeping leadership", "client_id", e.config.ClientID, "err", err)
}
}
}()
Expand All @@ -202,7 +202,7 @@ func (e *Elector) attemptGainLeadershipLoop(ctx context.Context) error {

numErrors++
sleepDuration := e.ExponentialBackoff(numErrors, baseservice.MaxAttemptsBeforeResetDefault)
e.Logger.Error(e.Name+": Error attempting to elect", "client_id", e.config.ClientID, "err", err, "num_errors", numErrors, "sleep_duration", sleepDuration)
e.Logger.ErrorContext(ctx, e.Name+": Error attempting to elect", "client_id", e.config.ClientID, "err", err, "num_errors", numErrors, "sleep_duration", sleepDuration)
e.CancellableSleep(ctx, sleepDuration)
continue
}
Expand Down Expand Up @@ -237,13 +237,13 @@ func (e *Elector) attemptGainLeadershipLoop(ctx context.Context) error {
func (e *Elector) handleLeadershipNotification(ctx context.Context, topic notifier.NotificationTopic, payload string) {
if topic != notifier.NotificationTopicLeadership {
// This should not happen unless the notifier is broken.
e.Logger.Error(e.Name+": Received unexpected notification", "client_id", e.config.ClientID, "topic", topic, "payload", payload)
e.Logger.ErrorContext(ctx, e.Name+": Received unexpected notification", "client_id", e.config.ClientID, "topic", topic, "payload", payload)
return
}

notification := dbLeadershipNotification{}
if err := json.Unmarshal([]byte(payload), &notification); err != nil {
e.Logger.Error(e.Name+": Unable to unmarshal leadership notification", "client_id", e.config.ClientID, "err", err)
e.Logger.ErrorContext(ctx, e.Name+": Unable to unmarshal leadership notification", "client_id", e.config.ClientID, "err", err)
return
}

Expand Down Expand Up @@ -291,7 +291,7 @@ func (e *Elector) keepLeadershipLoop(ctx context.Context) error {
// This doesn't use ctx because it runs *after* the ctx is done.
defer func() {
if !lostLeadership {
e.attemptResignLoop(ctx) // will resign using background context, but ctx sent for logging
e.attemptResignLoop(ctx) // will resign using WithoutCancel context, but ctx sent for logging
}
}()

Expand Down Expand Up @@ -373,16 +373,17 @@ func (e *Elector) attemptResignLoop(ctx context.Context) {
// still be elected.
const maxNumErrors = 3

// This does not inherit the parent context because we want to give up leadership
// even during a shutdown. There is no way to short-circuit this.
ctx = context.Background()
// This does not inherit the parent context's cancellation because we want to
// give up leadership even during a shutdown. There is no way to short-circuit
// this, though there are timeouts per call within attemptResign.
ctx = context.WithoutCancel(ctx)

for attempt := 1; attempt <= maxNumErrors; attempt++ {
if err := e.attemptResign(ctx, attempt); err != nil { //nolint:contextcheck
e.Logger.Error(e.Name+": Error attempting to resign", "attempt", attempt, "client_id", e.config.ClientID, "err", err)
e.Logger.ErrorContext(ctx, e.Name+": Error attempting to resign", "attempt", attempt, "client_id", e.config.ClientID, "err", err)

sleepDuration := e.ExponentialBackoff(attempt, baseservice.MaxAttemptsBeforeResetDefault)
e.Logger.Error(e.Name+": Error attempting to resign",
e.Logger.ErrorContext(ctx, e.Name+": Error attempting to resign",
"client_id", e.config.ClientID, "err", err, "num_errors", attempt, "sleep_duration", sleepDuration)
e.CancellableSleep(ctx, sleepDuration) //nolint:contextcheck

Expand Down