fix(supervisor): retry first launch on failure (#1538)

This commit is contained in:
Julien Neuhart
2026-04-27 19:05:05 +02:00
parent 68e0f88d5b
commit 64c28dd45e
2 changed files with 69 additions and 17 deletions

View File

@@ -86,12 +86,13 @@ type processSupervisor struct {
maxConcurrency int64
semaphore chan struct{}
firstStart atomic.Bool
firstStartOnce sync.Once
// firstStartErr stores the error from the first Launch attempt executed
// via firstStartOnce. Subsequent callers that enter the !firstStart block
// need to observe this value after the Once has completed, without
// re-executing the closure.
firstStartErr error
// firstStartMu serializes lazy-launch attempts so concurrent callers do
// not all spawn Launch() simultaneously. Using a mutex (instead of
// sync.Once) lets a failed launch be retried by the next caller, since a
// transient failure (such as a cold-start timeout) must not poison the
// supervisor for the rest of the container's lifetime. See
// https://github.com/gotenberg/gotenberg/issues/1538.
firstStartMu sync.Mutex
reqCounter atomic.Int64
reqQueueSize atomic.Int64
restartsCounter atomic.Int64
@@ -346,8 +347,6 @@ func (s *processSupervisor) maybeIdleShutdown() {
// Reset state so ensureStarted() re-launches on next request.
s.firstStart.Store(false)
s.firstStartOnce = sync.Once{}
s.firstStartErr = nil
s.reqCounter.Store(0)
s.logger.DebugContext(context.Background(), "process stopped due to idle timeout")
@@ -375,21 +374,27 @@ func (s *processSupervisor) acquireSlot(ctx context.Context, logger *slog.Logger
}
}
// ensureStarted performs a one-time lazy launch of the process on its first
// use. Subsequent calls are no-ops.
// ensureStarted performs a lazy launch of the process on its first use.
// Concurrent callers serialize on firstStartMu; once the launch succeeds,
// subsequent calls short-circuit on the firstStart flag. A failed launch
// leaves firstStart unset, so the next caller retries the launch.
func (s *processSupervisor) ensureStarted(ctx context.Context) error {
if s.firstStart.Load() {
return nil
}
s.firstStartOnce.Do(func() {
s.firstStartErr = s.runWithDeadline(ctx, func() error {
return s.Launch()
})
})
s.firstStartMu.Lock()
defer s.firstStartMu.Unlock()
if s.firstStartErr != nil {
return fmt.Errorf("process first start: %w", s.firstStartErr)
if s.firstStart.Load() {
return nil
}
err := s.runWithDeadline(ctx, func() error {
return s.Launch()
})
if err != nil {
return fmt.Errorf("process first start: %w", err)
}
return nil

View File

@@ -898,6 +898,53 @@ func TestProcessSupervisor_IdleShutdown(t *testing.T) {
}
}
func TestProcessSupervisor_RetryAfterFailedFirstStart(t *testing.T) {
// Regression test for https://github.com/gotenberg/gotenberg/issues/1538:
// a failed first launch must not poison the supervisor; the next request
// must retry Launch() instead of returning the cached error forever.
logger := slog.New(slog.DiscardHandler)
var startCalls atomic.Int64
process := &ProcessMock{
StartMock: func(logger *slog.Logger) error {
if startCalls.Add(1) == 1 {
return errors.New("first start failed")
}
return nil
},
StopMock: func(logger *slog.Logger) error {
return nil
},
HealthyMock: func(logger *slog.Logger) bool {
return true
},
}
ps := NewProcessSupervisor(logger, process, 0, 0, 1, 0).(*processSupervisor)
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
err := ps.Run(ctx, logger, func() error { return nil })
if err == nil {
t.Fatal("expected first Run to fail because Launch failed")
}
if ps.firstStart.Load() {
t.Fatal("firstStart must remain false after a failed Launch")
}
err = ps.Run(ctx, logger, func() error { return nil })
if err != nil {
t.Fatalf("expected second Run to succeed after the supervisor retries Launch, got: %v", err)
}
if !ps.firstStart.Load() {
t.Fatal("expected firstStart to be set after the second Launch succeeds")
}
if got := startCalls.Load(); got != 2 {
t.Fatalf("expected exactly 2 Start calls, got %d", got)
}
}
func TestProcessSupervisor_IdleShutdownSkippedWhenActive(t *testing.T) {
logger := slog.New(slog.DiscardHandler)