fix(supervisor): reset the request counter on a failed relaunch

restart() reset reqCounter only after a successful Launch, so a failed
one left it at the limit. maybeRestartAfterTask then re-fired on every
subsequent task, producing back-to-back restarts and, with planned
restarts now reporting healthy, a node that keeps restarting while
claiming health.

Reset on the attempt instead. A process that will not start is recovered
by ensureHealthy, which restarts synchronously and reports the failure.
This commit is contained in:
Julien Neuhart
2026-09-03 17:52:01 +02:00
parent d79e174c6f
commit 57b048c611
2 changed files with 42 additions and 1 deletions

View File

@@ -244,12 +244,18 @@ func (s *processSupervisor) restart() error {
s.logger.WarnContext(context.Background(), fmt.Sprintf("stop process before restart: %s", err)) s.logger.WarnContext(context.Background(), fmt.Sprintf("stop process before restart: %s", err))
} }
// Reset the counter on the attempt, not on its outcome. Leaving it at the
// limit after a failed launch re-triggers maybeRestartAfterTask on every
// subsequent task, producing back-to-back restarts. Recovering a process
// that will not start is ensureHealthy's job: it restarts synchronously
// before running a task, and reports the failure to the caller.
s.reqCounter.Store(0)
err = s.Launch() err = s.Launch()
if err != nil { if err != nil {
return fmt.Errorf("restart process: %w", err) return fmt.Errorf("restart process: %w", err)
} }
s.reqCounter.Store(0)
s.restartsCounter.Add(1) s.restartsCounter.Add(1)
s.logger.DebugContext(context.Background(), "process successfully restarted") s.logger.DebugContext(context.Background(), "process successfully restarted")

View File

@@ -160,6 +160,41 @@ func TestProcessSupervisor_restart(t *testing.T) {
} }
} }
// TestProcessSupervisor_restart_ResetsCounterOnFailedLaunch verifies that a
// restart whose launch fails still clears the request counter. Leaving it at
// the limit makes maybeRestartAfterTask re-fire on every subsequent task.
func TestProcessSupervisor_restart_ResetsCounterOnFailedLaunch(t *testing.T) {
logger := slog.New(slog.DiscardHandler)
const maxReqLimit = 5
process := &ProcessMock{
StartMock: func(_ *slog.Logger) error { return errors.New("start error") },
StopMock: func(_ *slog.Logger) error { return nil },
HealthyMock: func(_ *slog.Logger) bool { return true },
}
ps := NewProcessSupervisor(logger, "test", process, maxReqLimit, 0, 1, 0).(*processSupervisor)
ps.reqCounter.Store(maxReqLimit)
err := ps.restart()
if err == nil {
t.Fatal("expected error but got none")
}
if got := ps.reqCounter.Load(); got != 0 {
t.Fatalf("expected the request counter to be reset but got %d", got)
}
if got := ps.restartsCounter.Load(); got != 0 {
t.Fatalf("expected the restarts counter to stay at 0 but got %d", got)
}
if ps.maybeRestartAfterTask(logger) {
t.Fatal("expected no further eager restart to be triggered")
}
}
func TestProcessSupervisor_Healthy(t *testing.T) { func TestProcessSupervisor_Healthy(t *testing.T) {
for _, tc := range []struct { for _, tc := range []struct {
scenario string scenario string