carlos / platform Public

host: the restart check rides the serial, and seeds per app

REDESIGN (review): delete the 3s restart-generation sweep. The serial bump IS the latency mechanism — the console announces the touch like every other write, and the agent's existing 2s serial poll already turns a bump into the pass set, so the restart check becomes one more leg of fastTick (fastDeps.restartLeg, run after fullPass so processes cycle onto their new version and config rather than off them). Steady-state cost is now ZERO extra GETs; the loop it replaces was ~575k a day per box to notice an event that happens a few times a month. One read per app remains, at arming (restartSeed), so the first request after a boot is acted on instead of becoming the baseline. Direct-bucket touches that never bump keep the hourly RunAdopt floor.

STORM FIX (review, Critical): seed-don't-replay was file-level, so under a known baseline an app with no mark read as movement — a box whose registry landed after its marks file try-restarted EVERY route in one pass, and any new instance of a previously-restarted app got one spurious cycle. Marks now record a per-app absent sentinel, an unseen app is SEEDED rather than dispatched, a pass that sees zero adoptable routes writes nothing at all, and a baselining pass with any read error defers rather than persisting a baseline with holes in it.

Also: an exclusive flock around the whole pass (carlos-restart, carlos-stage and carlos-adopt can all run it at once — unserialized that is a double try-restart plus a marks clobber), with the lock file's creation doubling as the writability test that stops an unwritable /data from restarting the fleet hourly forever; and an honest coverage note that an activator-backed route carrying a -unit stamp is cycled by neither half.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell 9b4aec3f51ca7e3776fba57ad4590218978ae8ef parent ab7a90b
4 files changed, +569 −160
  • internal/host/agent.go +33 −1
  • internal/host/restart.go +267 −140
  • internal/host/restart_test.go +264 −14
  • internal/host/units/carlos-restart.path +5 −5
diff --git a/internal/host/agent.go b/internal/host/agent.go
index 36d9e9d4a..a6612ebfc 100644
--- a/internal/host/agent.go
+++ b/internal/host/agent.go
@@ -165,7 +165,6 @@ func RunAgent(args []string) {
go metricsLoop()
go configSyncLoop(reg, svc, dd)
go configGenLoop(reg, svc, dd)
- go restartGenLoop(reg, svc, dd)
go socketAdvisoryLoop(reg, svc)
// The same serving loop as `carlos edge`, with the activator injected.
@@ -720,6 +719,7 @@ type fastDeps struct {
restart func(string) error
passOpts []release.PassOption // fastPassOpts — every sweep pass carries all of them
fullPass func(ctx context.Context) // reconcile → adopt → config sync, or a test hook
+ restartLeg func(ctx context.Context) // the `carlos restart` check (issue #113), or a test hook
lastSerial int64
seeded bool
lastFull time.Time
@@ -856,6 +856,13 @@ func (d *fastDeps) backoffFor(host string) time.Duration { return d.step[host] }
// is deferred by NOT recording the acted serial, so the next tick past
// the window re-fires. Read errors are tolerated silently (one log
// line per state change, never per tick).
+// The restart leg (issue #113) rides that same move, AFTER the pass set:
+// a bump can carry a promote and a restart request at once, and cycling
+// processes before their new version and config land would cycle them onto
+// the old ones. It is a separate call rather than a fourth leg inside
+// fullPass because it is not release convergence — it restarts what is
+// already correct — and because it must still run on a box with no
+// CARLOS_SECRETS_KEY, which fullPass's config leg silently skips.
// 2. Fast sweep: fastMembers, and only when non-empty one PassFilter over
// exactly those hosts — an empty sweep costs zero store reads. After
// the sweep, members still unstaged back off; staged members clear
@@ -883,6 +890,9 @@ func fastTick(ctx context.Context, d *fastDeps, now time.Time) {
d.serialBroken = false
if doc.Serial != d.lastSerial && now.Sub(d.lastFull) >= fastFullClamp {
d.fullPass(ctx)
+ if d.restartLeg != nil {
+ d.restartLeg(ctx)
+ }
d.lastSerial = doc.Serial
d.lastFull = now
}
@@ -951,6 +961,13 @@ func fastConvergeLoop(reg *registry.Registry, svc *activator.Service) {
// Built once, used by both pass shapes below — see fastPassOpts for the
// DO-NOT list and why the stage requester is per-loop, not per-tick.
opts := fastPassOpts(dd, stageRequester(dd), restart)
+ // The restart check's memory (issue #113), seeded ONCE here — one read per
+ // app at arming and never again except on a serial move, which is what
+ // keeps an idle box at exactly one serial GET per tick. See restartSeed for
+ // why the baseline cannot wait for the first request to establish itself.
+ restartCache := map[[2]string]string{}
+ restartSeed(ctx, reg, store, dd, restartCache)
+ restartBounce := bounceFn(svc)
d := &fastDeps{
reg: reg, store: store, pub: pub, optDir: optDir,
restart: restart, passOpts: opts,
@@ -971,6 +988,17 @@ func fastConvergeLoop(reg *registry.Registry, svc *activator.Service) {
}
}
},
+ // The restart leg: this process's own children bounced here, everything
+ // unit-shaped relayed to a root pass. Deliberately NOT inside fullPass
+ // above — see fastTick's comment for both reasons.
+ restartLeg: func(ctx context.Context) {
+ routes, err := reg.List()
+ if err != nil {
+ log.Printf("agent: restart check: list routes: %v", err)
+ return
+ }
+ restartRequestLeg(ctx, routes, store, dd, restartBounce, restartCache)
+ },
}
log.Printf("agent: fast converge loop armed (2s + jitter: serial poll + canary/* + unstaged routes)")
for {
@@ -1416,6 +1444,10 @@ func statusLoop(reg *registry.Registry) {
// carlos-adopt.path is a box whose unit-backed config silently
// waits for the hourly timer (issue #44).
ConfigPathRunning: configPathActive(),
+ // A third, for the restart trigger: an enabled-but-unstarted
+ // carlos-restart.path is a box where `carlos restart` answers
+ // "requested" and nothing cycles until the hourly pass (issue #113).
+ RestartPathRunning: restartPathActive(),
// ExpiredCanaries answers spec §5 for this box's own routes; see
// countExpiredCanaries and the canaryStore/canaryPub comment above.
ExpiredCanaries: countExpiredCanaries(ctx, routes, canaryStore, canaryPub, time.Now()),
diff --git a/internal/host/restart.go b/internal/host/restart.go
index 0f206b725..7ebe61726 100644
--- a/internal/host/restart.go
+++ b/internal/host/restart.go
@@ -1,30 +1,47 @@
-// The box half of `carlos restart` (issue #113): watch each app's
-// restart-generation object (release.RestartGenKey) and, when one MOVES,
-// cycle that app's live processes on this box. Pull-based like everything
-// else here — no push channel, no box access, no version or config change
-// involved.
-//
-// The shape is config fast convergence's, one step sideways, and it is split
-// across the same two processes for the same reason:
-//
-// - the AGENT (carlos-edge.service, User=carlos, ProtectSystem=strict) can
-// read the bucket and see a request within seconds, and it owns the
-// EXEC-backed instances outright — so its half is the etag sweep, a bounce
-// of its own children, and one marker file for the rest.
+// The box half of `carlos restart` (issue #113): when an app's
+// restart-generation object (release.RestartGenKey) MOVES, cycle that app's
+// live processes on this box. Pull-based like everything else here — no push
+// channel, no box access, no version or config change involved.
+//
+// IT RIDES THE SERIAL, and that is the load-bearing design decision. The
+// console door bumps control/serial after touching the object, exactly as
+// every other console write does, and the agent's existing 2s serial poll
+// (fastTick) already turns a bump into the full convergence pass set. The
+// restart check is one more leg of THAT — so a box with nothing happening
+// pays its one serial GET per tick and NOT ONE READ MORE. An earlier revision
+// polled every app's restart generation on its own 3s timer; measured, that
+// was ~575k needless GETs per box per day to notice an event that arrives a
+// handful of times a month. A verb that costs the fleet a permanent poll storm
+// to exist is not a verb worth having.
+//
+// Direct-bucket touches that never bump the serial (a script writing the
+// object straight into the bucket) converge on the hourly root pass instead —
+// the same floor every other direct-bucket write already relies on.
+//
+// The work is split across two processes, for the reason config fast
+// convergence is split the same way:
+//
+// - the AGENT (carlos-edge.service, User=carlos, ProtectSystem=strict) owns
+// the EXEC-backed instances outright, so its half is the etag comparison,
+// a bounce of its own children, and one marker file for the rest.
+// systemctl cannot reach a child of another process, so a root-only
+// dispatch would leave every warm exec instance — the commonest shape on
+// the box, and the one a wedged instance usually is — serving on,
+// unrestarted.
// - the ROOT pass (carlos adopt, out of carlos-restart.service via
// carlos-restart.path, or the hourly timer) is the only one that can
// restart a systemd unit — the carlos-app@ tenants and the bespoke units
// addr-backed routes name (issue #110's consoles). Its half is the
-// comparison against a persisted high-water mark and the dispatch.
+// comparison against a persisted, per-app high-water mark and the
+// dispatch.
//
-// The split is exactly config fast convergence's: systemctl cannot reach a
-// child of another process, so a root-only dispatch would leave every warm
-// exec instance — the commonest shape on the box, and the one a wedged
-// instance usually is — serving on, unrestarted.
-//
-// SEED-DON'T-REPLAY governs both halves. A box that has never recorded a
-// baseline records one and restarts NOTHING: the first sight of an object is
-// never a request, or every upgrade would bounce the whole box.
+// SEED-DON'T-REPLAY governs both halves, PER APP. An app this box has not
+// recorded yet is recorded and NOT restarted, whether its object exists or
+// not; only movement away from a value this box has already written counts.
+// The absent case gets its own recorded sentinel precisely so that an app's
+// first ever request (absent -> present) is still movement rather than a first
+// sight — that distinction is the difference between the verb working and the
+// verb cycling a box's whole route table the day a marks file goes missing.
package host
import (
@@ -33,19 +50,18 @@ import (
"errors"
"fmt"
"log"
- "math/rand/v2"
"os"
"path/filepath"
+ "syscall"
"time"
- "github.com/carlosframework/platform/internal/activator"
"github.com/carlosframework/platform/internal/registry"
"github.com/carlosframework/platform/internal/release"
)
// restartApps lists the distinct (account, app) pairs with at least one
-// restartable route on this box — the apps whose restart generation is worth
-// one small GET per tick.
+// restartable route on this box — the apps whose restart generation this box
+// reads when a serial bump (or an hourly pass) gives it a reason to.
//
// The membership predicate is release.Adoptable, deliberately the SAME one the
// adopt pass moves and the fast sweep counts: a route a pass would not adopt
@@ -71,14 +87,16 @@ func restartApps(routes []registry.Route) [][2]string {
return out
}
-// restartGenPass is one tick of restartGenLoop, split out for tests: read the
+// restartGenPass is the agent's comparison, split out for tests: read the
// restart-generation etag of every app this box serves and report WHICH of
// them moved. Same semantics — and the same reasons — as genPollPass:
//
// - First sight of an app records silently and asks for nothing. A box
// coming up must not restart everything it serves because it has never
// seen these etags before; that is the seed-don't-replay rule, and it is
-// the whole safety property of this feature.
+// the whole safety property of this feature. restartSeed runs one such
+// silent pass at arming, so the FIRST request after a boot is acted on
+// rather than swallowed as the baseline.
// - An unchanged etag does nothing.
// - Absent that stays absent does nothing, FOREVER: an app nobody has ever
// restarted this way has no object and never will until someone asks.
@@ -103,8 +121,10 @@ func restartApps(routes []registry.Route) [][2]string {
// app is a first sight again, which is right, because a route that has
// just been (re-)minted is starting its process fresh anyway.
//
-// The cache is mutated in place — it lives across ticks in the loop, which is
-// the whole point (there is nowhere else to remember what "changed" means).
+// The cache is mutated in place and lives for the life of the agent process
+// (fastConvergeLoop owns it, restartSeed fills it), which is the whole point:
+// there is nowhere else to remember what "changed" means between two serial
+// moves that may be days apart.
func restartGenPass(ctx context.Context, routes []registry.Route, store release.Store, cache map[[2]string]string) (moved [][2]string, readErrs []error) {
apps := restartApps(routes)
live := make(map[[2]string]bool, len(apps))
@@ -137,10 +157,17 @@ func restartGenPass(ctx context.Context, routes []registry.Route, store release.
// half no root pass can perform: an exec instance is a child of THIS process,
// and systemctl has no name for it.
//
-// The route filter is the complement of what rootRestartRoute can reach — no
-// named unit, no unit backing — so the two halves cannot cycle the same
-// process twice, and every restartable route is covered by exactly one of
-// them.
+// The route filter is the complement of what rootRestartRoute's systemctl arms
+// can reach — no named unit, no unit backing — so the two halves never cycle
+// the same process twice.
+//
+// Coverage is NOT total, and the gap is worth naming: a route that is
+// activator-backed (Backing "", an exec child) but ALSO carries a Unit stamp
+// is skipped here (it names a unit) while the root half try-restarts that
+// named unit, which is not the process the activator is running. Such a route
+// is a misconfiguration — `carlos route -unit` is for addr-backed tenants
+// whose process an operator owns — but nothing in this file repairs it, and
+// its exec child is cycled by NEITHER half until somebody clears the stamp.
//
// HIBERNATE-flagged routes are included, and that is not a contradiction of
// "never wake a sleeper": Bounce provably cannot start anything (it is a no-op
@@ -176,8 +203,8 @@ func bounceExecRoutes(routes []registry.Route, moved [][2]string, bounce func(ho
return n
}
-// restartRequestMarker is the file restartGenLoop touches to ask a root pass
-// to run now: carlos-restart.path watches it (PathChanged) and starts
+// restartRequestMarker is the file the agent's restart leg touches to ask a
+// root pass to run now: carlos-restart.path watches it (PathChanged) and starts
// carlos-restart.service (`carlos adopt -restart-only`), which owns the half
// this unprivileged process cannot do — restarting units — and only that half.
//
@@ -214,6 +241,26 @@ func restartMarksPath(dd string) string { return filepath.Join(dd, restartMarksF
// outside every character class a sqid or an app name allows, the same
// separator argument internal/release's bake clock makes for its own keys.
//
+// A value is either the etag this box last acted on OR restartAbsent, the
+// recorded "this app has no restart generation and I have looked". On disk:
+//
+// {
+// "schema": 1,
+// "apps": {
+// "bab|keymail": "\"9f86d0…\"",
+// "bab|titogo": "\u0000absent"
+// }
+// }
+//
+// The absent sentinel is what makes per-app seed-don't-replay possible. Without
+// it "no entry" has to mean both "never looked" and "looked, nothing there",
+// and one of those two has to be wrong: read as movement, a box gaining a new
+// route for a previously-restarted app cycles it for a request served weeks
+// ago (and a box whose registry lands after its marks file cycles EVERY route
+// at once); read as a first sight, an app's first ever request is silently
+// dropped. Recording the absence tells them apart — a later object sighting is
+// movement from a value this box wrote, which is exactly what it is.
+//
// Entries are never pruned when a route leaves the box. A vanished app's mark
// costs one map entry; DROPPING it would make the app's return look like a
// brand-new object and cycle it once for a request that was already served.
@@ -222,8 +269,53 @@ type restartMarks struct {
Apps map[string]string `json:"apps"`
}
+// restartAbsent is the marks value for an app this box has SEEN and found to
+// have no restart-generation object at all. It is deliberately the SAME value
+// the in-memory sweep uses (genAbsent), so the agent's memory and the root
+// pass's memory cannot drift apart on what "absent" is, and it is safe for the
+// same reason: no store produces an etag containing a NUL byte (S3's are
+// hex/quoted, DirStore's is hex). JSON renders it "\u0000absent".
+const restartAbsent = genAbsent
+
func restartMarkKey(acct, app string) string { return acct + "|" + app }
+// lockRestartMarks takes an exclusive flock for the whole of one restartPass.
+// Three root passes can run this — carlos-restart.service (the relay),
+// carlos-stage.service and carlos-adopt.service — and systemd will happily
+// start them at the same second. Unserialized, two passes read the same marks,
+// both dispatch (one request, two try-restarts, on a verb whose entire job is
+// stopping processes) and then both write, so whichever finishes last silently
+// reverts the other's advance and the NEXT pass restarts everything a third
+// time.
+//
+// The lock lives in a sibling <marks>.lock rather than on the marks file
+// itself, because the marks file is replaced by rename: a flock is held on an
+// inode, and the winner's rename would leave the loser holding a lock on a file
+// that is no longer the marks file.
+//
+// Opening it is ALSO the writability test the dispatch is gated on (see
+// restartPass): it is created in the same directory, with the same permissions
+// the marks save needs, so a full or read-only /data fails here — before
+// anything is cycled — instead of after, which is the difference between one
+// logged refusal and an hourly restart of every app on the box forever.
+//
+// syscall.Flock is available on both linux and darwin, which is the whole
+// build matrix (CI cross-compiles darwin because the CLI runs on Macs).
+func lockRestartMarks(path string) (unlock func(), err error) {
+ f, err := os.OpenFile(path+".lock", os.O_CREATE|os.O_RDWR, 0o644)
+ if err != nil {
+ return nil, err
+ }
+ if err := syscall.Flock(int(f.Fd()), syscall.LOCK_EX); err != nil {
+ f.Close()
+ return nil, err
+ }
+ return func() {
+ syscall.Flock(int(f.Fd()), syscall.LOCK_UN)
+ f.Close()
+ }, nil
+}
+
// loadRestartMarks reads the marks file. `known` is the seed-don't-replay
// switch and the only subtle thing here: it is FALSE when the file is missing,
// unreadable, or does not parse — three different accidents with one correct
@@ -300,21 +392,31 @@ func (m *restartMarks) save(path string) error {
}
// seedRestartMarks writes the baseline the ROOT pass compares against, ONCE,
-// from the agent's first sweep — create-only (O_EXCL), so it can never
+// from the agent's arming sweep — create-only (O_EXCL), so it can never
// overwrite a mark the root pass has already advanced.
//
// It exists to close a real hole in the handover between the two halves. The
-// agent seeds its own baseline within seconds of starting; the root pass would
-// otherwise not seed until it next ran, and a restart requested inside that
-// window would arrive at a pass with no baseline — which, by the rule above,
-// records and restarts nothing. The request would be silently dropped, having
-// told its caller "requested". Creating the file here shrinks that window to
-// the agent's first tick after boot, where the box has just restarted
-// everything anyway.
-//
-// Absent objects are NOT recorded: an app with no restart generation has
-// nothing to baseline, and its object's later appearance is a genuine request.
+// agent seeds its own baseline as it arms; the root pass would otherwise not
+// seed until it next ran, and a restart requested inside that window would
+// arrive at a pass with no baseline — which, by the rule above, records and
+// restarts nothing. The request would be silently dropped, having told its
+// caller "requested". Creating the file here shrinks that window to the moment
+// the agent starts, where the box has just restarted everything anyway.
+//
+// An EMPTY cache writes nothing at all. A sweep that saw no adoptable routes
+// is not a baseline — most often it is a box whose registry has not landed yet
+// (a fresh machine, or one restoring /data from litestream) — and a file
+// claiming "this box knows about no apps" is precisely the file that makes the
+// root pass treat every route it later gains as movement, and cycle the lot in
+// one pass.
+//
+// Absent objects ARE recorded, with restartAbsent: "looked, nothing there" is
+// the fact that makes an app's first ever request movement rather than a first
+// sight.
func seedRestartMarks(path string, cache map[[2]string]string) error {
+ if len(cache) == 0 {
+ return nil // nothing was seen; nothing may be claimed
+ }
if _, err := os.Stat(path); err == nil {
return nil // the root pass (or an earlier agent) already has a baseline
} else if !errors.Is(err, os.ErrNotExist) {
@@ -322,9 +424,6 @@ func seedRestartMarks(path string, cache map[[2]string]string) error {
}
m := &restartMarks{Schema: 1, Apps: map[string]string{}}
for k, etag := range cache {
- if etag == genAbsent {
- continue
- }
m.Apps[restartMarkKey(k[0], k[1])] = etag
}
raw, err := json.MarshalIndent(m, "", " ")
@@ -355,13 +454,24 @@ func seedRestartMarks(path string, cache map[[2]string]string) error {
//
// The rules, each a deliberate choice:
//
-// - An app with NO restart generation is skipped entirely and records
-// nothing: absent is legal forever.
-// - An unknown baseline (see loadRestartMarks) records and dispatches
-// nothing. Seed-don't-replay, at the only layer that could cycle a whole
-// box at once.
-// - A missing mark for ONE app under a known baseline IS movement: the
-// object exists, so somebody asked, and this box has never acted on it.
+// - ZERO adoptable routes ends the pass before anything is read or written.
+// A box whose registry has not landed yet (fresh, or restoring /data) must
+// not be able to record "I know about no apps" — that file is what turns
+// every route the box later gains into movement, and cycles the whole
+// table in one pass.
+// - An app this box has NOT recorded yet is recorded and NOT cycled,
+// whether its object exists or not. Per-app seed-don't-replay: the marks
+// file is this box's memory of what it has acted on, and a request it
+// never saw is not one it owes anybody.
+// - An app with NO restart generation records restartAbsent — the sentinel
+// that makes its FIRST ever request (absent -> present) movement rather
+// than another first sight. Absent is legal forever; unrecorded absence is
+// what was not.
+// - An unknown baseline (see loadRestartMarks) records every app and
+// dispatches nothing — and if any app could not be READ, records nothing
+// at all: a baseline missing the apps a transient 503 hid is a baseline
+// that turns those apps into movement on the next pass. A dropped request
+// is recoverable (run the verb again); a restart nobody asked for is not.
// - HIBERNATING routes are skipped outright and not marked as a failure.
// Restarting a process that is not running is meaningless, waking it to
// restart it is worse (it would defeat hibernation for no gain), and it
@@ -376,34 +486,58 @@ func seedRestartMarks(path string, cache map[[2]string]string) error {
// retry.
// - The mark is written only AFTER the dispatch, so a pass that dies
// mid-cycle retries rather than swallowing the request.
+// - The whole pass holds an exclusive lock (lockRestartMarks), because three
+// root units can run it at once; failing to TAKE that lock dispatches
+// nothing, since the lock file's creation is also the proof that the marks
+// can be saved afterwards. A pass that cycles processes and then cannot
+// record what it did would cycle them again every hour, forever.
//
// Holds, bake windows and the release ladder take no part in any of this:
// nothing about which version runs changes here, so there is nothing for them
// to gate.
func restartPass(ctx context.Context, routes []registry.Route, store release.Store, marksPath string, restart func(host string) error) {
+ apps := restartApps(routes)
+ if len(apps) == 0 {
+ return // nothing to converge, and nothing this box may claim to know
+ }
+ unlock, err := lockRestartMarks(marksPath)
+ if err != nil {
+ // No lock means either a concurrent pass we cannot serialize against or
+ // a /data we cannot write. Both make dispatching the wrong move: the
+ // first double-restarts, the second restarts hourly and forever because
+ // the mark can never advance.
+ log.Printf("adopt: restart pass: cannot lock %s.lock (%v) — nothing cycled this pass", marksPath, err)
+ return
+ }
+ defer unlock()
+
marks, known := loadRestartMarks(marksPath)
changed := false
- for _, k := range restartApps(routes) {
+ readFailed := false
+ for _, k := range apps {
_, etag, err := store.Get(ctx, release.RestartGenKey(k[0], k[1]))
- if errors.Is(err, os.ErrNotExist) {
- continue // never asked for: nothing to record, nothing to do
- }
- if err != nil {
+ switch {
+ case errors.Is(err, os.ErrNotExist):
+ etag = restartAbsent
+ case err != nil:
log.Printf("adopt: restart generation %s/%s: %v (skipped this pass)", k[0], k[1], err)
+ readFailed = true
continue
}
key := restartMarkKey(k[0], k[1])
prev, seen := marks.Apps[key]
- if !known {
- // Baseline only: this box has no memory it can trust, so it
- // records what the bucket says and cycles nothing.
+ if !known || !seen {
+ // SEED. Either this box's whole memory is untrustworthy, or this is
+ // the first time it has looked at this app — a route it has just
+ // gained, whose process is starting fresh anyway. Record, cycle
+ // nothing.
if !seen || prev != etag {
marks.Apps[key] = etag
changed = true
}
continue
}
- if seen && prev == etag {
+ if prev == etag {
continue
}
dispatched := 0
@@ -432,6 +566,14 @@ func restartPass(ctx context.Context, routes []registry.Route, store release.Sto
marks.Apps[key] = etag
changed = true
}
+ if !known && readFailed {
+ // A baseline that omits the apps a blip hid is worse than no baseline:
+ // this pass would write "known", and the next one would read those
+ // missing apps as movement and cycle them. Leave the file as it is and
+ // re-baseline on a pass that can read everything.
+ log.Printf("adopt: restart pass: baselining deferred — some apps could not be read this pass")
+ return
+ }
// A first, unknown baseline is written even when nothing changed: that
// write is what makes the NEXT pass a known one, and until it lands every
// pass would re-baseline and drop whatever it was asked to do.
@@ -442,82 +584,67 @@ func restartPass(ctx context.Context, routes []registry.Route, store release.Sto
}
}
-// restartGenLoop is the agent's half: every ~3s, read one small object per app
-// this box serves and, when one moves, bounce that app's exec children here
-// and touch the marker that asks a root pass to cycle everything else. The
-// tick's whole decision lives in restartGenPass and bounceExecRoutes, which
-// are tested; this is the thin plumbing around them, like adoptLoop and
-// configGenLoop.
-//
-// Deliberately NOT folded into configGenLoop, though it is the same shape:
-// that loop is gated on CARLOS_SECRETS_KEY (without the key it cannot render
-// an app's environment at all, so it would poll to signal no one). Restarting
-// needs no secrets key, no release key and no signature — so gating it on one
-// would mean a box that happens to carry no secrets key silently has no
-// restart verb, and the person typing `carlos restart` would be told
-// "requested" by a console that was right, about a box that would never act.
-//
-// Deliberately NOT doing here:
-//
-// - No fast first pass. The loop sleeps BEFORE its first tick and the first
-// sight of every app is silent (see restartGenPass): a box coming up must
-// not cycle everything it serves, and it has in any case just started all
-// of it.
-// - No unit restarting, and no marks bookkeeping. This process writes no
-// /etc and runs no systemctl: for anything unit-shaped it only touches
-// <dataDir>/restart-requested, which carlos-restart.path relays into a
-// root `carlos adopt -restart-only` run. That pass owns the marks file,
-// and this loop's in-memory cache is deliberately a SECOND, independent
-// memory — the two halves cycle disjoint sets of routes, so neither can
-// mark the other's work done.
-func restartGenLoop(reg *registry.Registry, svc *activator.Service, dd string) {
- ctx := context.Background()
- store, err := openDeployStore(ctx)
+// restartRequestLeg is the agent's half of one serial move: compare each app's
+// restart generation against the in-memory cache, bounce this process's own
+// exec children for the apps that moved, and touch the marker that asks a root
+// pass to cycle everything unit-shaped. Reports whether it asked.
+//
+// It runs ONLY when the serial moved (fastTick), which is what makes the
+// feature free at rest: the console bumps the serial after touching the
+// object, so every request is announced, and an idle box reads no restart
+// generation at all. There is no loop of its own and there must not be one —
+// see this file's package comment for what the 3s version of this cost.
+//
+// The bounce goes first, and not only for tidiness: it is in-process and
+// instant, while the marker write can block on a busy /data, and the children
+// bounced here are the ones cycled here or nowhere.
+func restartRequestLeg(ctx context.Context, routes []registry.Route, store release.Store, dd string,
+ bounce func(host string) bool, cache map[[2]string]string) bool {
+ moved, readErrs := restartGenPass(ctx, routes, store, cache)
+ if len(readErrs) > 0 {
+ // ONE line per pass, never one per app: the count carries the scale,
+ // the first error the cause — they are the same cause in practice.
+ log.Printf("agent: restart check: %d apps unreadable: %v", len(readErrs), readErrs[0])
+ }
+ if len(moved) == 0 {
+ return false
+ }
+ bounceExecRoutes(routes, moved, bounce)
+ if err := requestRootRestart(dd); err != nil {
+ log.Printf("agent: request root restart pass: %v", err)
+ return false
+ }
+ log.Printf("agent: restart requested for %d app(s) on this box, asked for a root restart pass", len(moved))
+ return true
+}
+
+// restartSeed is the ONE sweep this agent makes on its own initiative: at
+// arming, read each app's restart generation once, silently, so the cache the
+// leg compares against exists before the first serial move — and hand the same
+// sightings to seedRestartMarks so the root pass has a baseline too.
+//
+// Without it the first request after a boot is the sweep that establishes the
+// baseline, and is therefore the one request that does nothing: the person who
+// typed `carlos restart` would have to type it twice, having been told
+// "requested" the first time. One read per app per agent start is the whole
+// cost, and it is paid where a box has just restarted everything anyway.
+//
+// A read error leaves that app out of the cache (its next sighting is a first
+// sight — safe) and blocks the marks seed entirely, for the reason
+// seedRestartMarks and restartPass both give: a baseline with holes in it is
+// the thing that manufactures restarts.
+func restartSeed(ctx context.Context, reg *registry.Registry, store release.Store, dd string, cache map[[2]string]string) {
+ routes, err := reg.List()
if err != nil {
- log.Printf("agent: restart request loop disabled: %v", err)
+ log.Printf("agent: restart baseline: list routes: %v (the first request may need a second try)", err)
return
}
- bounce := bounceFn(svc)
- // One cache for the life of the process: what "changed" means is exactly
- // "differs from the last tick that could read it".
- cache := map[[2]string]string{}
- seeded := false
- log.Printf("agent: restart request loop armed (3s + jitter)")
- for {
- time.Sleep(3*time.Second + rand.N(time.Second))
- routes, err := reg.List()
- if err != nil {
- log.Printf("agent: restart poll: list routes: %v", err)
- continue
- }
- moved, readErrs := restartGenPass(ctx, routes, store, cache)
- if len(readErrs) > 0 {
- // ONE line per pass, never one per app: a bucket having a bad
- // minute would otherwise flood the journal at 20 lines a minute per
- // app. The count carries the scale, the first error the cause.
- log.Printf("agent: restart poll: %d apps unreadable: %v", len(readErrs), readErrs[0])
- }
- if !seeded && len(readErrs) == 0 {
- // After the first CLEAN pass, whose sightings are the baseline by
- // definition. A pass that could not read some app is not a
- // baseline: seeding from it would leave that app unrecorded, and
- // an unrecorded app under a known baseline reads as a request.
- // Create-only; see seedRestartMarks.
- if err := seedRestartMarks(restartMarksPath(dd), cache); err != nil {
- log.Printf("agent: seed restart marks: %v", err)
- }
- seeded = true
- }
- if len(moved) == 0 {
- continue
- }
- // This process's own children first — they are cycled here or nowhere,
- // and the write below can block on a busy /data.
- bounceExecRoutes(routes, moved, bounce)
- if err := requestRootRestart(dd); err != nil {
- log.Printf("agent: request root restart pass: %v", err)
- continue
- }
- log.Printf("agent: restart requested for %d app(s) on this box, asked for a root restart pass", len(moved))
+ _, readErrs := restartGenPass(ctx, routes, store, cache)
+ if len(readErrs) > 0 {
+ log.Printf("agent: restart baseline: %d apps unreadable: %v", len(readErrs), readErrs[0])
+ return
+ }
+ if err := seedRestartMarks(restartMarksPath(dd), cache); err != nil {
+ log.Printf("agent: seed restart marks: %v", err)
}
}
diff --git a/internal/host/restart_test.go b/internal/host/restart_test.go
index 48c203d9c..bf59e7a98 100644
--- a/internal/host/restart_test.go
+++ b/internal/host/restart_test.go
@@ -7,7 +7,9 @@ import (
"os"
"path/filepath"
"slices"
+ "sync"
"testing"
+ "time"
"github.com/carlosframework/platform/internal/registry"
"github.com/carlosframework/platform/internal/release"
@@ -243,6 +245,91 @@ func TestBounceExecRoutesWithoutAnActivator(t *testing.T) {
}
}
+// ---- the agent's leg, and what makes it run ----------------------------------
+
+// TestRestartRequestLegBouncesAndRelays: one serial move's worth of work — the
+// agent's own children cycled in-process, and one marker touched for the units
+// only root can reach.
+func TestRestartRequestLegBouncesAndRelays(t *testing.T) {
+ ctx, s := genKit(t)
+ dd := t.TempDir()
+ marker := filepath.Join(dd, restartRequestMarker)
+ routes := []registry.Route{
+ execRoute("exec.example.com", "bab", "eleven"),
+ unitRoute("unit.example.com", "bab", "eleven"),
+ }
+ var bounced []string
+ bounce := func(h string) bool { bounced = append(bounced, h); return true }
+ cache := map[[2]string]string{}
+
+ // Baseline (what restartSeed does at arming): silent, no marker.
+ askRestart(t, ctx, s, "bab", "eleven")
+ if asked := restartRequestLeg(ctx, routes, s, dd, bounce, cache); asked {
+ t.Fatal("the baselining leg asked for a root pass — first sight must be silent")
+ }
+ if _, err := os.Stat(marker); !errors.Is(err, os.ErrNotExist) {
+ t.Fatalf("marker written on a first sight (err %v)", err)
+ }
+
+ askRestart(t, ctx, s, "bab", "eleven")
+ if asked := restartRequestLeg(ctx, routes, s, dd, bounce, cache); !asked {
+ t.Fatal("a moved restart generation must ask for a root pass")
+ }
+ if !slices.Equal(bounced, []string{"exec.example.com"}) {
+ t.Fatalf("bounced %v, want the exec child only (the unit route is root's)", bounced)
+ }
+ if _, err := os.Stat(marker); err != nil {
+ t.Fatalf("no marker after a request: %v", err)
+ }
+
+ // And a leg with nothing moved neither bounces nor re-asks.
+ bounced = nil
+ if asked := restartRequestLeg(ctx, routes, s, dd, bounce, cache); asked {
+ t.Fatal("an unmoved generation asked for a root pass")
+ }
+ if len(bounced) != 0 {
+ t.Fatalf("bounced %v with nothing moved", bounced)
+ }
+}
+
+// TestFastTickRunsTheRestartLegOnASerialMove pins the wiring the redesign rests
+// on: the restart check rides the SERIAL, which is what keeps an idle box at
+// one GET per tick. A boot seed must not run it (that would cycle processes for
+// bumps that happened while the box was down), an unchanged serial must not run
+// it, and a real move must — once, after the pass set.
+func TestFastTickRunsTheRestartLegOnASerialMove(t *testing.T) {
+ env := newFastFixture(t)
+ var passes, legs int
+ d := env.deps(func() { passes++ })
+ d.restartLeg = func(context.Context) {
+ legs++
+ if legs > passes {
+ t.Errorf("the restart leg ran before/without the pass set (legs=%d passes=%d)", legs, passes)
+ }
+ }
+ now := time.Unix(1754640000, 0)
+ ctx := context.Background()
+
+ if err := release.BumpSerial(ctx, env.store, now); err != nil {
+ t.Fatalf("BumpSerial: %v", err)
+ }
+ fastTick(ctx, d, now) // first sight of a present serial: seed only
+ if legs != 0 {
+ t.Fatalf("boot seed ran the restart leg %d times, want 0", legs)
+ }
+ fastTick(ctx, d, now.Add(10*time.Second)) // unchanged
+ if legs != 0 {
+ t.Fatalf("unchanged serial ran the restart leg %d times, want 0", legs)
+ }
+ if err := release.BumpSerial(ctx, env.store, now); err != nil {
+ t.Fatalf("BumpSerial: %v", err)
+ }
+ fastTick(ctx, d, now.Add(20*time.Second))
+ if legs != 1 {
+ t.Fatalf("a serial move ran the restart leg %d times, want 1", legs)
+ }
+}
+
// ---- the root pass -----------------------------------------------------------
func marksOf(t *testing.T, path string) *restartMarks {
@@ -327,11 +414,12 @@ func TestRestartPassCyclesOnMovement(t *testing.T) {
}
}
-// TestRestartPassActsOnAnAppsFirstEverRequest: under a KNOWN baseline, an app
-// with no mark at all is movement, not a first sight. It has to be: the object
-// only comes into existence when somebody asks, so treating "no mark yet" as
-// something to seed would drop the first `carlos restart` for every app,
-// forever.
+// TestRestartPassActsOnAnAppsFirstEverRequest: an app with no object yet is
+// recorded as ABSENT, and the object's later appearance is movement away from
+// that recorded value. Both halves matter — without the recorded absence, the
+// first `carlos restart` for every app is either dropped (if "no mark" seeds)
+// or joined by a spurious cycle of every other app in the same state (if "no
+// mark" dispatches).
func TestRestartPassActsOnAnAppsFirstEverRequest(t *testing.T) {
ctx, s := genKit(t)
routes := []registry.Route{unitRoute("a.example.com", "bab", "eleven")}
@@ -339,8 +427,8 @@ func TestRestartPassActsOnAnAppsFirstEverRequest(t *testing.T) {
rec := &recordingRestart{}
restartPass(ctx, routes, s, path, rec.fn) // baseline: the app has no object at all
- if m := marksOf(t, path); len(m.Apps) != 0 {
- t.Fatalf("marks = %v, want no entry for an app with no restart generation", m.Apps)
+ if got := marksOf(t, path).Apps[restartMarkKey("bab", "eleven")]; got != restartAbsent {
+ t.Fatalf("mark = %q, want the recorded absent sentinel", got)
}
askRestart(t, ctx, s, "bab", "eleven")
@@ -350,6 +438,152 @@ func TestRestartPassActsOnAnAppsFirstEverRequest(t *testing.T) {
}
}
+// TestRestartPassSeedsAnAppItHasNeverSeen is the storm fix (review, Critical).
+// A route added to this box for an app that was restarted somewhere else weeks
+// ago must be RECORDED, not cycled: this box owes nothing to a request it never
+// saw, and reading "no mark" as movement is what turned a registry that landed
+// after the marks file into a box that try-restarted its entire route table in
+// one pass.
+func TestRestartPassSeedsAnAppItHasNeverSeen(t *testing.T) {
+ ctx, s := genKit(t)
+ askRestart(t, ctx, s, "bab", "eleven")
+ askRestart(t, ctx, s, "bab", "titogo") // both restarted long ago, elsewhere
+ first := []registry.Route{unitRoute("a.example.com", "bab", "eleven")}
+ path := filepath.Join(t.TempDir(), "restart-marks.json")
+ rec := &recordingRestart{}
+ restartPass(ctx, first, s, path, rec.fn) // this box knows about eleven only
+
+ // titogo's route arrives now — a fresh instance, mid-provision until this
+ // moment. Its app's object has existed for weeks.
+ both := append(first, unitRoute("b.example.com", "bab", "titogo"))
+ restartPass(ctx, both, s, path, rec.fn)
+ if len(rec.hosts) != 0 {
+ t.Fatalf("a newly-seen app cycled %v, want nothing — this box never saw that request", rec.hosts)
+ }
+ if got, want := marksOf(t, path).Apps[restartMarkKey("bab", "titogo")], restartEtag(t, ctx, s, "bab", "titogo"); got != want {
+ t.Fatalf("mark = %q, want the seeded etag %q", got, want)
+ }
+ // And it is live from here: the NEXT request cycles it.
+ askRestart(t, ctx, s, "bab", "titogo")
+ restartPass(ctx, both, s, path, rec.fn)
+ if !slices.Equal(rec.hosts, []string{"b.example.com"}) {
+ t.Fatalf("cycled %v, want the newly-seeded app's route on the next real request", rec.hosts)
+ }
+}
+
+// TestRestartPassWithNoRoutesWritesNothing: a box whose registry has not landed
+// yet (fresh machine, /data restoring from litestream) must not be able to
+// record "I know about no apps" — that file is what makes every route it later
+// gains look like movement.
+func TestRestartPassWithNoRoutesWritesNothing(t *testing.T) {
+ ctx, s := genKit(t)
+ askRestart(t, ctx, s, "bab", "eleven")
+ path := filepath.Join(t.TempDir(), "restart-marks.json")
+ rec := &recordingRestart{}
+
+ restartPass(ctx, nil, s, path, rec.fn)
+ if _, err := os.Stat(path); !errors.Is(err, os.ErrNotExist) {
+ t.Fatalf("a pass with zero adoptable routes wrote %s (err %v) — it knows nothing and may claim nothing", path, err)
+ }
+ // The registry lands; the app is seeded, not cycled.
+ routes := []registry.Route{unitRoute("a.example.com", "bab", "eleven")}
+ restartPass(ctx, routes, s, path, rec.fn)
+ if len(rec.hosts) != 0 {
+ t.Fatalf("cycled %v after the registry landed, want a silent baseline", rec.hosts)
+ }
+}
+
+// TestRestartPassDefersABaselineWithReadErrors: a baseline that omits the apps
+// a transient 503 hid is a baseline that turns those apps into movement on the
+// next pass. Nothing is written until a pass can read every app.
+func TestRestartPassDefersABaselineWithReadErrors(t *testing.T) {
+ ctx, s := genKit(t)
+ askRestart(t, ctx, s, "bab", "eleven")
+ askRestart(t, ctx, s, "bab", "titogo")
+ routes := []registry.Route{
+ unitRoute("a.example.com", "bab", "eleven"),
+ unitRoute("b.example.com", "bab", "titogo"),
+ }
+ path := filepath.Join(t.TempDir(), "restart-marks.json")
+ rec := &recordingRestart{}
+
+ blip := &failingKeyStore{Store: s, failKey: release.RestartGenKey("bab", "eleven"), err: errors.New("503 slow down")}
+ restartPass(ctx, routes, blip, path, rec.fn)
+ if _, err := os.Stat(path); !errors.Is(err, os.ErrNotExist) {
+ t.Fatalf("a baselining pass with a read error wrote %s (err %v)", path, err)
+ }
+
+ // A clean pass baselines both, and cycles neither.
+ restartPass(ctx, routes, s, path, rec.fn)
+ if len(rec.hosts) != 0 {
+ t.Fatalf("the clean baselining pass cycled %v, want nothing", rec.hosts)
+ }
+ if m := marksOf(t, path); len(m.Apps) != 2 {
+ t.Fatalf("marks = %v, want both apps", m.Apps)
+ }
+}
+
+// TestRestartPassRefusesWithoutALock: no lock means either a concurrent root
+// pass or a /data this pass cannot write. Dispatching under the first
+// double-restarts one request; under the second it restarts hourly forever,
+// since the mark can never advance. Neither is allowed to happen.
+func TestRestartPassRefusesWithoutALock(t *testing.T) {
+ ctx, s := genKit(t)
+ askRestart(t, ctx, s, "bab", "eleven")
+ dir := t.TempDir()
+ path := filepath.Join(dir, "restart-marks.json")
+ routes := []registry.Route{unitRoute("a.example.com", "bab", "eleven")}
+ rec := &recordingRestart{}
+ restartPass(ctx, routes, s, path, rec.fn) // baseline, with a working lock
+
+ // A directory where the lock file needs to be: opening it fails (EISDIR),
+ // exactly as opening it would fail on a read-only or full /data.
+ if err := os.Remove(path + ".lock"); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.Mkdir(path+".lock", 0o755); err != nil {
+ t.Fatal(err)
+ }
+ askRestart(t, ctx, s, "bab", "eleven")
+ restartPass(ctx, routes, s, path, rec.fn)
+ if len(rec.hosts) != 0 {
+ t.Fatalf("cycled %v with no lock — an unrecordable restart repeats forever", rec.hosts)
+ }
+}
+
+// TestRestartPassSerializesConcurrentPasses: three root units can run this at
+// once (carlos-restart, carlos-stage, carlos-adopt). Under the lock, one
+// request produces ONE dispatch however many passes race for it.
+func TestRestartPassSerializesConcurrentPasses(t *testing.T) {
+ ctx, s := genKit(t)
+ askRestart(t, ctx, s, "bab", "eleven")
+ path := filepath.Join(t.TempDir(), "restart-marks.json")
+ routes := []registry.Route{unitRoute("a.example.com", "bab", "eleven")}
+ var mu sync.Mutex
+ var hosts []string
+ dispatch := func(h string) error {
+ mu.Lock()
+ defer mu.Unlock()
+ hosts = append(hosts, h)
+ return nil
+ }
+ restartPass(ctx, routes, s, path, dispatch) // baseline
+
+ askRestart(t, ctx, s, "bab", "eleven")
+ var wg sync.WaitGroup
+ for range 3 {
+ wg.Add(1)
+ go func() {
+ defer wg.Done()
+ restartPass(ctx, routes, s, path, dispatch)
+ }()
+ }
+ wg.Wait()
+ if len(hosts) != 1 {
+ t.Fatalf("three concurrent passes dispatched %v, want exactly one — the flock is what makes a request singular", hosts)
+ }
+}
+
// TestRestartPassAdvancesTheMarkOnAFailedDispatch: a restart request is a
// point-in-time event, not a desired state. A named unit that will not come
// back must be reported, not retried on every pass until the end of time.
@@ -398,22 +632,24 @@ func TestRestartPassUnreadableMarksReBaseline(t *testing.T) {
// ---- the handover between the two halves -------------------------------------
-// TestSeedRestartMarksIsCreateOnly: the agent writes the baseline within
-// seconds of starting so a request made before the root pass has ever run is
-// not silently dropped — but it must never overwrite a mark the root pass has
-// already advanced, or a served request would be served again.
+// TestSeedRestartMarksIsCreateOnly: the agent writes the baseline as it arms,
+// so a request made before the root pass has ever run is not silently dropped
+// — but it must never overwrite a mark the root pass has already advanced, or
+// a served request would be served again. Absent sightings ARE recorded: that
+// sentinel is what makes an app's first ever request movement.
func TestSeedRestartMarksIsCreateOnly(t *testing.T) {
path := filepath.Join(t.TempDir(), "restart-marks.json")
cache := map[[2]string]string{
{"bab", "eleven"}: "etag-1",
- {"bab", "titogo"}: genAbsent, // never asked for: nothing to baseline
+ {"bab", "titogo"}: genAbsent, // seen, and it has no object
}
if err := seedRestartMarks(path, cache); err != nil {
t.Fatalf("seed: %v", err)
}
m := marksOf(t, path)
- if len(m.Apps) != 1 || m.Apps[restartMarkKey("bab", "eleven")] != "etag-1" {
- t.Fatalf("seeded marks = %v, want only the app that has an object", m.Apps)
+ if m.Apps[restartMarkKey("bab", "eleven")] != "etag-1" ||
+ m.Apps[restartMarkKey("bab", "titogo")] != restartAbsent {
+ t.Fatalf("seeded marks = %v, want the etag and the recorded absence", m.Apps)
}
cache[[2]string{"bab", "eleven"}] = "etag-2"
@@ -425,6 +661,20 @@ func TestSeedRestartMarksIsCreateOnly(t *testing.T) {
}
}
+// TestSeedRestartMarksRefusesAnEmptyBaseline: a sweep that saw nothing is most
+// often a box whose registry has not landed yet. A file claiming "this box
+// knows about no apps" is exactly what makes the root pass treat every route
+// the box later gains as movement.
+func TestSeedRestartMarksRefusesAnEmptyBaseline(t *testing.T) {
+ path := filepath.Join(t.TempDir(), "restart-marks.json")
+ if err := seedRestartMarks(path, map[[2]string]string{}); err != nil {
+ t.Fatalf("seed: %v", err)
+ }
+ if _, err := os.Stat(path); !errors.Is(err, os.ErrNotExist) {
+ t.Fatalf("an empty sweep wrote %s (err %v), want no file at all", path, err)
+ }
+}
+
// TestSeededBaselineIsWhatTheRootPassTrusts closes the handover: the agent
// seeds, a member then asks, and the root pass — running for the FIRST time,
// with no memory of its own — still cycles. Without the seed that pass would
diff --git a/internal/host/units/carlos-restart.path b/internal/host/units/carlos-restart.path
index e735d94db..f5e20875d 100644
--- a/internal/host/units/carlos-restart.path
+++ b/internal/host/units/carlos-restart.path
@@ -1,10 +1,10 @@
# Member-requested process restarts (issue #113) — carlos-adopt.path's twin
# for the third half the sandboxed agent cannot do itself. The agent's
-# restartGenLoop sees an app's restart generation move within seconds and
-# bounces its OWN exec children immediately, but it cannot restart a systemd
-# unit: not a carlos-app@ tenant, and not the bespoke unit an addr-backed route
-# names (issue #110's consoles). Touching /data/restart-requested relays that
-# to a root pass.
+# serial poll sees the console's bump within ~2s, compares the app's restart
+# generation, and bounces its OWN exec children immediately — but it cannot
+# restart a systemd unit: not a carlos-app@ tenant, and not the bespoke unit an
+# addr-backed route names (issue #110's consoles). Touching
+# /data/restart-requested relays that to a root pass.
#
# It starts carlos-restart.service — `carlos adopt -restart-only` —
# deliberately NOT carlos-adopt.service or carlos-config-sync.service. A