Manager: fix error in sleep scheduler when shutting down
When the Manager was shutting down while the sleep scheduler was running, it could cause a null pointer dereference. This is now doubly solved: - `worker.Identifier()` is now nil-safe, as in, `worker` can be `nil` and it will still return a sensible string. - failure to apply the sleep schedule due to the context closing is not logged as error any more.
This commit is contained in:
parent
50ec5f4f36
commit
59655ea770
@ -33,6 +33,10 @@ type Worker struct {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func (w *Worker) Identifier() string {
|
func (w *Worker) Identifier() string {
|
||||||
|
// Avoid a panic when worker.Identifier() is called on a nil pointer.
|
||||||
|
if w == nil {
|
||||||
|
return "-nil worker-"
|
||||||
|
}
|
||||||
return fmt.Sprintf("%s (%s)", w.Name, w.UUID)
|
return fmt.Sprintf("%s (%s)", w.Name, w.UUID)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -4,6 +4,7 @@ package sleep_scheduler
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
@ -200,7 +201,12 @@ func (ss *SleepScheduler) CheckSchedules(ctx context.Context) {
|
|||||||
func (ss *SleepScheduler) checkSchedule(ctx context.Context, schedule *persistence.SleepSchedule) {
|
func (ss *SleepScheduler) checkSchedule(ctx context.Context, schedule *persistence.SleepSchedule) {
|
||||||
// Compute the next time to check.
|
// Compute the next time to check.
|
||||||
schedule.NextCheck = ss.calculateNextCheck(schedule)
|
schedule.NextCheck = ss.calculateNextCheck(schedule)
|
||||||
if err := ss.persist.SetWorkerSleepScheduleNextCheck(ctx, schedule); err != nil {
|
err := ss.persist.SetWorkerSleepScheduleNextCheck(ctx, schedule)
|
||||||
|
switch {
|
||||||
|
case errors.Is(ctx.Err(), context.Canceled):
|
||||||
|
// Manager is shutting down, this is fine.
|
||||||
|
return
|
||||||
|
case err != nil:
|
||||||
log.Error().
|
log.Error().
|
||||||
Err(err).
|
Err(err).
|
||||||
Str("worker", schedule.Worker.Identifier()).
|
Str("worker", schedule.Worker.Identifier()).
|
||||||
@ -209,12 +215,15 @@ func (ss *SleepScheduler) checkSchedule(ctx context.Context, schedule *persisten
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Apply the schedule to the worker.
|
// Apply the schedule to the worker.
|
||||||
if err := ss.ApplySleepSchedule(ctx, schedule); err != nil {
|
err = ss.ApplySleepSchedule(ctx, schedule)
|
||||||
|
switch {
|
||||||
|
case errors.Is(ctx.Err(), context.Canceled):
|
||||||
|
// Manager is shutting down, this is fine.
|
||||||
|
case err != nil:
|
||||||
log.Error().
|
log.Error().
|
||||||
Err(err).
|
Err(err).
|
||||||
Str("worker", schedule.Worker.Identifier()).
|
Str("worker", schedule.Worker.Identifier()).
|
||||||
Msg("sleep scheduler: error applying worker's sleep schedule")
|
Msg("sleep scheduler: error applying worker's sleep schedule")
|
||||||
return
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -95,6 +95,25 @@ func TestSetScheduleSwappedStartEnd(t *testing.T) {
|
|||||||
assert.NoError(t, err)
|
assert.NoError(t, err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Test that a sleep check that happens at shutdown of the Manager doesn't cause any panics.
|
||||||
|
func TestCheckSleepScheduleAtShutdown(t *testing.T) {
|
||||||
|
ss, mocks, _ := testFixtures(t)
|
||||||
|
|
||||||
|
sched := persistence.SleepSchedule{
|
||||||
|
IsActive: true,
|
||||||
|
DaysOfWeek: "mo tu we",
|
||||||
|
StartTime: mkToD(18, 0),
|
||||||
|
EndTime: mkToD(9, 0),
|
||||||
|
Worker: nil,
|
||||||
|
}
|
||||||
|
|
||||||
|
// Cancel the context to mimick the Manager shutting down.
|
||||||
|
ctx, cancel := context.WithCancel(context.Background())
|
||||||
|
cancel()
|
||||||
|
mocks.persist.EXPECT().SetWorkerSleepScheduleNextCheck(ctx, &sched).Return(context.Canceled)
|
||||||
|
ss.checkSchedule(ctx, &sched)
|
||||||
|
}
|
||||||
|
|
||||||
func TestApplySleepSchedule(t *testing.T) {
|
func TestApplySleepSchedule(t *testing.T) {
|
||||||
ss, mocks, ctx := testFixtures(t)
|
ss, mocks, ctx := testFixtures(t)
|
||||||
|
|
||||||
|
Loading…
x
Reference in New Issue
Block a user