Skip to content

Commit 47b54d4

Browse files
frahlgclaude
andauthored
fix(drivers): retry a driver that fails to start (#1890)
* fix(drivers): retry a driver that fails to start A driver whose Add fails (a serial node not enumerated yet, a device still booting after a power cut) was logged once and forgotten, so a missing P1 meter blocked dispatch until a restart or a settings save. The registry now retries it after 10 s, doubling to 5 min, until it starts, leaves the config, is removed, or the registry shuts down. Running drivers are never torn down, and probes are not retried. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(drivers): wait for an in-flight spawn retry in Reload and ShutdownAll add() deletes the retry entry before the driver reaches r.rec, so while a retry ran its name was in neither list Reload checks. A driver the user removed or disabled could then start, or schedule yet another retry. ShutdownAll could also return while the retry was still starting a driver. The retry now marks its name in flight under r.mu for the length of add. Reload sees the mark, takes the name's gate and removes what the retry left. ShutdownAll waits on the gate of each in-flight name before it lists drivers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent d35748e commit 47b54d4

5 files changed

Lines changed: 469 additions & 1 deletion

File tree

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
"ftw": patch
3+
---
4+
5+
Retry a driver that fails to start, for example a USB P1 meter that is not
6+
ready at boot. FTW tries again after 10 s and doubles the wait up to 5 min
7+
until the driver starts, leaves the config or is removed. Before, the site
8+
stayed without control until a restart or a settings save.

‎go/internal/drivers/registry.go‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,9 @@ type Registry struct {
122122
lifecycleHook func(name string)
123123
lifecycleMu sync.Mutex
124124
lifecycleGates map[string]*lifecycleGate
125+
spawnRetries map[string]*spawnRetry
126+
spawnInflight map[string]struct{}
127+
spawnRetryClosed bool
125128
}
126129

127130
// lifecycleGate serializes every lifecycle transition for one driver name.
@@ -536,7 +539,7 @@ func legacyDriverRequiresDefaultMode(path string, hasCommand bool) (bool, error)
536539
// add is the shared driver construction path. The caller must hold the
537540
// lifecycle gate for cfg.Name. startupDefault is false only for connection
538541
// probes; operational drivers use the startup safety gate.
539-
func (r *Registry) add(ctx context.Context, cfg config.Driver, startupDefault bool) error {
542+
func (r *Registry) add(ctx context.Context, cfg config.Driver, startupDefault bool) (err error) {
540543
if ctx == nil {
541544
ctx = context.Background()
542545
}
@@ -546,6 +549,15 @@ func (r *Registry) add(ctx context.Context, cfg config.Driver, startupDefault bo
546549
return fmt.Errorf("driver %q already registered", cfg.Name)
547550
}
548551
r.mu.Unlock()
552+
if startupDefault {
553+
prevDelay := r.takeSpawnRetry(cfg.Name)
554+
retryCfg := cfg
555+
defer func() {
556+
if err != nil {
557+
r.scheduleSpawnRetry(retryCfg, prevDelay, err)
558+
}
559+
}()
560+
}
549561

550562
if cfg.Lua == "" {
551563
return fmt.Errorf("driver %q: must specify `lua` path", cfg.Name)
@@ -1206,6 +1218,7 @@ func (r *Registry) remove(name string, skipDefault bool) {
12061218
// lets Restart and Reload keep the old generation and its replacement in one
12071219
// serialized transition instead of opening an Add/Remove gap.
12081220
func (r *Registry) removeLocked(name string, skipDefault bool) {
1221+
r.takeSpawnRetry(name)
12091222
skipDefault = skipDefault || r.controlBlocked()
12101223
r.mu.Lock()
12111224
rd, ok := r.rec[name]
@@ -1441,6 +1454,11 @@ func (r *Registry) Names() []string {
14411454

14421455
// ShutdownAll stops every driver. Blocks until all poll loops exit.
14431456
func (r *Registry) ShutdownAll() {
1457+
// An attempt in flight holds its gate and tears its driver down itself
1458+
// once it sees the registry closed. Wait for it.
1459+
for _, n := range r.closeSpawnRetries() {
1460+
r.acquireLifecycle(n)()
1461+
}
14441462
r.mu.Lock()
14451463
names := make([]string, 0, len(r.rec))
14461464
for n := range r.rec {
@@ -1467,6 +1485,14 @@ func (r *Registry) Reload(ctx context.Context, newDrivers []config.Driver, troub
14671485
}
14681486
active = append(active, d)
14691487
}
1488+
// Stop retrying a driver that failed to start and has left the config.
1489+
for _, n := range r.pendingSpawnRetries() {
1490+
if _, ok := findDriver(active, n); !ok {
1491+
release := r.acquireLifecycle(n)
1492+
r.removeLocked(n, false)
1493+
release()
1494+
}
1495+
}
14701496

14711497
r.mu.Lock()
14721498
troubleshootingChanged := r.troubleshootingMode != troubleshootingMode
Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
package drivers
2+
3+
import (
4+
"context"
5+
"log/slog"
6+
"time"
7+
8+
"github.com/srcfl/ftw/go/internal/config"
9+
)
10+
11+
// A driver whose start fails (a serial node not yet enumerated, a device
12+
// still booting after a power cut) is retried with this backoff until it
13+
// starts, leaves the config, or the registry shuts down. Running drivers are
14+
// never torn down here.
15+
const (
16+
spawnRetryMinDelay = 10 * time.Second
17+
spawnRetryMaxDelay = 5 * time.Minute
18+
)
19+
20+
// spawnRetry is one pending retry. The map entry is the claim: an Add,
21+
// Restart, Reload or Remove for the same name replaces or deletes it under the
22+
// lifecycle gate, so a timer that fires late finds a different entry and stops.
23+
type spawnRetry struct {
24+
cfg config.Driver
25+
delay time.Duration
26+
attempting bool
27+
timer *time.Timer
28+
}
29+
30+
// takeSpawnRetry removes the pending retry for name and returns the delay the
31+
// next failure should double, or 0 when this attempt did not come from the
32+
// retry timer. The caller holds the lifecycle gate for name.
33+
func (r *Registry) takeSpawnRetry(name string) time.Duration {
34+
r.mu.Lock()
35+
defer r.mu.Unlock()
36+
e := r.spawnRetries[name]
37+
if e == nil {
38+
return 0
39+
}
40+
delete(r.spawnRetries, name)
41+
e.timer.Stop()
42+
if e.attempting {
43+
return e.delay
44+
}
45+
return 0
46+
}
47+
48+
func (r *Registry) scheduleSpawnRetry(cfg config.Driver, prev time.Duration, cause error) {
49+
delay := spawnRetryMinDelay
50+
if prev > 0 {
51+
delay = min(2*prev, spawnRetryMaxDelay)
52+
}
53+
r.mu.Lock()
54+
defer r.mu.Unlock()
55+
if r.spawnRetryClosed {
56+
return
57+
}
58+
if r.spawnRetries == nil {
59+
r.spawnRetries = make(map[string]*spawnRetry)
60+
}
61+
e := &spawnRetry{cfg: cfg, delay: delay}
62+
e.timer = time.AfterFunc(delay, func() { r.retrySpawn(cfg.Name, e) })
63+
r.spawnRetries[cfg.Name] = e
64+
slog.Warn("driver failed to start; will retry", "name", cfg.Name, "retry_in", delay, "err", cause)
65+
}
66+
67+
func (r *Registry) retrySpawn(name string, e *spawnRetry) {
68+
release := r.acquireLifecycle(name)
69+
defer release()
70+
r.mu.Lock()
71+
if r.spawnRetries[name] != e {
72+
r.mu.Unlock()
73+
return
74+
}
75+
e.attempting = true
76+
// add deletes the map entry before the driver reaches r.rec. Until add
77+
// returns, the in-flight mark is what lets Reload and ShutdownAll find
78+
// this name and wait on its gate.
79+
if r.spawnInflight == nil {
80+
r.spawnInflight = make(map[string]struct{})
81+
}
82+
r.spawnInflight[name] = struct{}{}
83+
r.mu.Unlock()
84+
err := r.add(context.Background(), e.cfg, true)
85+
r.mu.Lock()
86+
delete(r.spawnInflight, name)
87+
closed := r.spawnRetryClosed
88+
r.mu.Unlock()
89+
if err != nil {
90+
return
91+
}
92+
if closed {
93+
// Shutdown began while this attempt ran and may have missed it.
94+
r.removeLocked(name, false)
95+
return
96+
}
97+
slog.Info("driver started on retry", "name", name)
98+
}
99+
100+
// pendingSpawnRetries lists names with a scheduled retry or one in flight.
101+
func (r *Registry) pendingSpawnRetries() []string {
102+
r.mu.Lock()
103+
defer r.mu.Unlock()
104+
out := make([]string, 0, len(r.spawnRetries)+len(r.spawnInflight))
105+
for n := range r.spawnRetries {
106+
out = append(out, n)
107+
}
108+
for n := range r.spawnInflight {
109+
if _, dup := r.spawnRetries[n]; !dup {
110+
out = append(out, n)
111+
}
112+
}
113+
return out
114+
}
115+
116+
// closeSpawnRetries stops every pending retry, refuses new ones and returns
117+
// the names whose attempt is still in flight.
118+
func (r *Registry) closeSpawnRetries() []string {
119+
r.mu.Lock()
120+
defer r.mu.Unlock()
121+
r.spawnRetryClosed = true
122+
for _, e := range r.spawnRetries {
123+
e.timer.Stop()
124+
}
125+
r.spawnRetries = nil
126+
inflight := make([]string, 0, len(r.spawnInflight))
127+
for n := range r.spawnInflight {
128+
inflight = append(inflight, n)
129+
}
130+
return inflight
131+
}
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
package drivers
2+
3+
import (
4+
"context"
5+
"errors"
6+
"slices"
7+
"testing"
8+
"time"
9+
10+
"github.com/srcfl/ftw/go/internal/config"
11+
"github.com/srcfl/ftw/go/internal/telemetry"
12+
)
13+
14+
// blockingSerialRegistry fails the first open, then blocks every later open
15+
// until the test sends a result.
16+
func blockingSerialRegistry(t *testing.T) (*Registry, chan struct{}, chan error) {
17+
t.Helper()
18+
entered := make(chan struct{}, 4)
19+
result := make(chan error)
20+
first := true
21+
r := NewRegistry(telemetry.NewStore())
22+
r.SerialFactory = func(name string, c *config.SerialConfig) (SerialCap, error) {
23+
if first {
24+
first = false
25+
return nil, errors.New("no such device")
26+
}
27+
entered <- struct{}{}
28+
if err := <-result; err != nil {
29+
return nil, err
30+
}
31+
return fakeSerial{}, nil
32+
}
33+
t.Cleanup(r.ShutdownAll)
34+
return r, entered, result
35+
}
36+
37+
// startInflightRetry fails a boot Add, then runs its retry until it blocks
38+
// in the serial open. The returned channel closes when the retry returns.
39+
func startInflightRetry(t *testing.T, r *Registry, cfg config.Driver, entered chan struct{}) chan struct{} {
40+
t.Helper()
41+
if err := r.Add(context.Background(), cfg); err == nil {
42+
t.Fatal("expected first Add to fail")
43+
}
44+
e := pendingSpawnRetry(r, cfg.Name)
45+
if e == nil {
46+
t.Fatal("no retry scheduled")
47+
}
48+
e.timer.Stop()
49+
done := make(chan struct{})
50+
go func() { r.retrySpawn(cfg.Name, e); close(done) }()
51+
select {
52+
case <-entered:
53+
case <-time.After(5 * time.Second):
54+
t.Fatal("retry did not reach the serial open")
55+
}
56+
return done
57+
}
58+
59+
// returnsWithin reports whether done closes within d.
60+
func returnsWithin(done chan struct{}, d time.Duration) bool {
61+
select {
62+
case <-done:
63+
return true
64+
case <-time.After(d):
65+
return false
66+
}
67+
}
68+
69+
// The user disables a failing driver while a retry runs, and the retry then
70+
// succeeds. The disabled driver must not be left running.
71+
func TestReloadDuringInflightRetrySuccessStopsDriver(t *testing.T) {
72+
r, entered, result := blockingSerialRegistry(t)
73+
cfg := serialRetryDriver(t)
74+
done := startInflightRetry(t, r, cfg, entered)
75+
76+
reloadDone := make(chan struct{})
77+
go func() {
78+
disabled := cfg
79+
disabled.Disabled = true
80+
r.Reload(context.Background(), []config.Driver{disabled}, false)
81+
close(reloadDone)
82+
}()
83+
if returnsWithin(reloadDone, 300*time.Millisecond) {
84+
t.Error("Reload returned while the retry attempt was still in flight")
85+
}
86+
result <- nil
87+
<-done
88+
<-reloadDone
89+
if slices.Contains(r.Names(), "p1") {
90+
t.Fatalf("driver disabled in config is running after an in-flight retry: %v", r.Names())
91+
}
92+
if e := pendingSpawnRetry(r, "p1"); e != nil {
93+
t.Fatalf("retry scheduled for a disabled driver: %+v", e)
94+
}
95+
}
96+
97+
// Same, but the retry fails. No further retry may be scheduled.
98+
func TestReloadDuringInflightRetryFailureStopsRetrying(t *testing.T) {
99+
r, entered, result := blockingSerialRegistry(t)
100+
cfg := serialRetryDriver(t)
101+
done := startInflightRetry(t, r, cfg, entered)
102+
103+
reloadDone := make(chan struct{})
104+
go func() {
105+
r.Reload(context.Background(), nil, false)
106+
close(reloadDone)
107+
}()
108+
returnsWithin(reloadDone, 300*time.Millisecond)
109+
result <- errors.New("still missing")
110+
<-done
111+
<-reloadDone
112+
if e := pendingSpawnRetry(r, "p1"); e != nil {
113+
t.Fatalf("retry still scheduled (in %v) for a driver removed from config", e.delay)
114+
}
115+
}
116+
117+
// ShutdownAll blocks until all poll loops exit, so it must wait for a retry
118+
// that may still start a driver.
119+
func TestShutdownAllWaitsForInflightRetry(t *testing.T) {
120+
r, entered, result := blockingSerialRegistry(t)
121+
cfg := serialRetryDriver(t)
122+
done := startInflightRetry(t, r, cfg, entered)
123+
124+
shutDone := make(chan struct{})
125+
go func() { r.ShutdownAll(); close(shutDone) }()
126+
if returnsWithin(shutDone, 300*time.Millisecond) {
127+
t.Error("ShutdownAll returned while a retry attempt was still starting the driver")
128+
}
129+
result <- nil
130+
<-done
131+
<-shutDone
132+
if slices.Contains(r.Names(), "p1") {
133+
t.Fatalf("driver running after shutdown: %v", r.Names())
134+
}
135+
}

0 commit comments

Comments
 (0)