Long-run durability and execution isolation - #27
Merged
Conversation
First-class artifact entity for Dispatch: Trove-backed object storage with declared job inputs, imperative outputs, a content-addressed worker-local staging cache with disk budget, and two-phase lifecycle sweeping scoped to ephemeral artifacts only. Foundation for the heavy-workload track (resource model, execution isolation, long-run durability, resource prediction).
18 tasks across 6 phases: entity and five store backends, Backend interface and Trove adapter, staging cache, middleware and handler API, Forge extension wiring, and the two-phase sweeper.
The suite's SweepNeverTouchesDurable case encodes the design's safety invariant: no sweep path may mark a durable artifact, whatever its age, links, or owner state.
Sweep eligibility keeps lifecycle = 'ephemeral' as a SQL literal in every statement, so durable artifacts are unreachable from the sweep paths regardless of caller input.
Full conformance suite passes against a real SQL engine, including the SweepNeverTouchesDurable invariant.
Store.Migrate builds indexes from migrationIndexes rather than the migrate group, so the artifact indexes -- including the partial unique index on live storage keys -- go there. Full conformance suite passes against MongoDB.
Completes phase 1. The ephemeral sorted set is Redis's form of the SQL lifecycle literal: durable artifacts are never members, so the sweeps cannot reach them, and the lifecycle is re-checked on load and again on write. Conformance suite passes on all five backends.
Register stats but deliberately does not hash: hashing a multi-gigabyte input would turn enqueue into a full read pass. The hash is filled in later by the staging cache, which already streams every byte. Ephemeral keys embed the attempt so a retried job writing the same name cannot collide with its own previous attempt.
Two behaviours the tests forced out of the naive implementation: Trove drivers do not consistently wrap the package not-found sentinels (memdriver returns a bare fmt.Errorf), so translate carries a documented substring fallback. Without it a deleted input would be classified as transient and burn every retry instead of failing fast to the DLQ. Range reads are a driver capability, not a Get option: a driver without it silently returns the whole object. OpenRange now type-asserts RangeDriver and reports ErrRangeUnsupported rather than handing back more bytes than asked for.
Hashing happens during the download rather than as a separate pass, so the content_hash an artifact carries as NULL after registration fills itself in at no extra cost. Single-flight collapses concurrent stages of the same artifact into one download; leases pin an entry so eviction cannot pull a file out from under a running handler; the byte budget bounds disk use and is the artifact plane's first piece of admission control. A request larger than the whole budget fails immediately rather than blocking until its deadline -- no amount of eviction could satisfy it.
Staging runs as ordinary middleware, so the executor is untouched. That is also the right boundary for out-of-process execution later: staging happens outside it, and a sandboxed handler receives a directory of files rather than storage credentials. Bindings travel in their own job field rather than inside Payload, because the payload is opaque to the engine -- refs buried there would be invisible to both the scheduler and this middleware. Leases are released through a defer that survives a handler panic; the budget in the tests is sized so a leak would deadlock the third run.
RegisterChecked validates a definition's declarations against the staging budget, so a job that could never be staged fails at registration rather than on every worker that picks it up. Enqueue validates bindings against declarations before persisting, which keeps an oversized or undeclared input out of the retry path entirely. The job registry now carries input specs alongside handlers, because by execution time the typed definition is gone and the staging middleware only has a job name.
Three-tier resolution mirroring resolveGroveDB: an explicit backend, a named Trove store from config, then the default instance in the container. Trove's extension registers *trove.Trove both unnamed and per named store, so multi-store setups work without importing trove/extension. Enabling artifacts but mounting no Trove is an error rather than a silent no-op -- the operator asked for a feature and should hear that it could not be provided.
Deletion is two-phase: a sweep marks and stops serving, a later purge removes bytes after a grace window. A mistaken sweep is observable and reversible for the length of that window rather than instantly destructive, and both phases are idempotent under retry. The ephemeral-only guarantee is enforced twice -- the store's queries constrain themselves with a literal, and the sweeper re-checks every artifact before acting. A property test over random create/fail/retry/ sweep sequences asserts no durable artifact is ever touched. A backend failure skips that artifact rather than aborting the pass, so one unreachable object cannot stall reclamation of everything else.
setupTestStore still built a *bun.DB, which postgres.New stopped accepting when the store moved to the grove ORM. The whole postgres_test package therefore failed to compile, so every integration test in it had been dead code that never ran.
NewRaw does not translate ? placeholders -- the existing job store uses $1/$2 -- so UpdateArtifact, ListArtifactsByOwner, both sweeps, ListPurgeable, and PurgeArtifact all raised a syntax error against a real database. Also passes NULL rather than the empty string for the nullable hash and content-type columns on update. Caught by the artifact conformance suite once the postgres integration harness compiled again; all 14 cases now pass against Postgres 16.
AcquireLeadership projected a single id column but scanned into the full 10-field worker model, which the driver rejects. Leader election on Postgres therefore failed outright, taking distributed cron scheduling with it -- and, since the artifact sweeper is leader-gated, reclamation too. The bug was invisible because TestClusterStore_Leadership lives in the integration test package that had stopped compiling.
TestJobStore_DequeueSkipLocked stamped run_at from this process's clock and then immediately dequeued, where eligibility is run_at <= NOW() evaluated against the database's clock. The two disagree by a fraction of a millisecond when Postgres runs in a container, so the most recently enqueued jobs sat briefly in the future and were skipped. That made the test fail two different ways across runs -- 0 jobs dequeued, and priority 1 returned ahead of priority 2 -- because how many rows the predicate admitted depended on how far the clocks had drifted. Instrumenting it showed 0 or 1 of 3 rows eligible where the test assumed all 3. Backdating run_at keeps the test about priority ordering and SKIP LOCKED. The store is deliberately left alone: run_at <= NOW() is the correct semantics for a scheduler, and widening it to absorb clock skew would let jobs fire early.
The package migrated from the bun ORM to grove, but 69 wrapped-error strings still named the old layer. Align them with the "dispatch/postgres:" prefix already used by artifact.go and wake.go. No behavior change.
Replace the 90 repeated "dispatch/postgres: " literals with a single errPrefix const in helpers.go, concatenated into each format string. Constant concatenation keeps the format argument a compile-time constant, so go vet's printf analysis still validates verbs and arity -- verified by injecting an arity error and confirming vet reports it identically in both the concatenated and literal forms. No behavior change.
Nothing imports github.com/uptrace/bun since store/postgres moved to grove, so tidy drops it along with its transitive deps: jinzhu/inflection, mellium.im/sasl, tmthrgd/go-hex and puzpuzpuz/xsync. bunrouter is a separate module and is unaffected. Also promotes zeebo/blake3 from indirect to direct -- artifact/cache imports it directly.
Track C of the heavy-workload track. Defines an exec.Executor abstraction that generalizes today's in-process handler call, with four implementations forming an escalating ladder: in-process, subprocess, OCI, and Kubernetes Job-per-task. Key decisions: - Insertion point is the terminal closure in worker/executor.go, so track A's staging middleware keeps running in the host process and the sandbox receives a directory rather than storage credentials. - Handlers reach the sandbox by re-exec of the same binary, which has the same registry by construction. job.Registrable (a method on a generic type) is the seam that lets heterogeneous definitions reach a credential-free entrypoint. - The handler holds no credentials at any rung. In Kubernetes that means a three-container pod: an init container stages inputs, a native sidecar uploads outputs, and the handler container has no token and no network. - Deterministic Job names fence against double-launch after a reap. - Launch failures requeue without consuming the job's retry budget.
Track B of the heavy-workload track. Replaces identical worker slots with a weighted resource model: - resource.Set as map[string]int64 in canonical units, resolved to a concrete set at enqueue and written to the job row so scheduling never calls user code. - resource.Manager generalizing artifact/cache/budget.go to N keys, with the cache registering as the disk Reclaimer rather than keeping a second budget system. - job.Store.DequeueJobs widened to DequeueOpts so the fit predicate lives in the query; claim-then-requeue would thrash exactly the heavy jobs this exists for. - Reservation with backfill bounded by job.Timeout, which is enforced and therefore an upper bound rather than a prediction. No track E dependency. - Per-run measurement plus a bounded (job_name, input_bucket) rollup that ships as the non-ML default estimator behind the interface track E later implements.
Track A shipped and track B's spec landed after this document was written. Three corrections, each of which would otherwise have become an implementation bug: - Resources: exec.Resources and ResourceResolver are superseded by track B's resource.Spec / SpecFrom(ctx). Track C resolves nothing; the spec is resolved at enqueue and read from context. Adds the canonical-unit to corev1 mapping table and the reverse obligation, C supplying resource.Sampler. - The shim accessor: artifact.Accessor.Create returns a concrete *artifact.CommitWriter, so the shim builds a real *artifact.Service over a localfs Backend and an in-memory Store rather than reimplementing the interface. Handler code cannot tell which side of the boundary it is on. - Resumption: Existing/IfAbsent would silently break out-of-process, since prior attempts' links live in the store the shim cannot reach, and a re-rendered page is still correct output so no test would catch it. Request.PriorOutputs now carries them, resolved by the worker.
Phase 1 of four: the exec.Executor abstraction, the in-process rung, the job.Registrable seam, and the conformance suite. No behaviour change and no new dependencies. Ten tasks, each ending in a tested, committable deliverable. The plan is written against the code as it actually is rather than as the spec sketched it, which caught four signature errors during review, the substantive one being that engine.Register returns nothing while RegisterChecked already returns an error. The policy check goes in RegisterChecked beside the artifact validation, so nothing breaks.
exec.WithIsolation passed straight to NewDefinition would have forced exec to import job, breaking the leaf constraint stated in section 3. Follows the pattern track A already established: artifact.Input returns a value and job.WithArtifactInputs adapts it.
…stale claims Real bug: prepareOutputDir creates a "dispatch-out-…" scratch directory for any out-of-process attempt regardless of whether the Runner has an artifact plane configured (WithArtifacts' own doc comment already said so), but sweepStaleScratchDirs returned immediately when r.artifacts was nil — so a Runner with no artifact plane leaked exactly the directories it also creates, and Reclaim's startup sweep could never find them. Removed the gate; the sweep is safe to run unconditionally because it only ever touches entries matching scratchDirPrefix with a well-formed embedded PID. Replaced TestRunner_ReclaimDoesNotSweepWithoutAnArtifactPlane, which had pinned the old behaviour as intentional, with TestRunner_ReclaimSweepsWithoutAnArtifactPlaneToo, which also checks a live-owner directory still survives with no artifact plane configured. Comment/doc corrections surfaced along the way, each checked against current HEAD: - Runner.Reclaim, sweepStaleScratchDirs, and engine.Build's runner.WithArtifacts call all asserted scratch-dir creation was gated on the artifact plane; it never was — only PriorOutputs and committing are. engine.WithScratchRoot's doc pointed at "Build's own comment at the call site" for a config-time-warning rationale that had moved to extension/execution.go's resolveExecutionOptions. - staleScratchDirAge's doc credited itself with protecting a live sibling's directory; that protection is processAlive(pid), with age only a courtesy tie-break once ownership already says the PID is dead — sweepStaleScratchDirs' own doc already said this correctly. - artifact.Service's doc claimed an artifact row is never written without its link; Register writes a durable row with a nil link, which Store.CreateArtifact's contract supports. Scoped the invariant to the CommitWriter path. - Runner.Execute's outcome enumeration missed a launch failure with retries remaining (StatePending, no event), a permanent failure DLQing on its first attempt, and abandonLostLease writing nothing at all to the store. - pool.go's defaultStoreCallTimeout and WithPollInterval docs were sized against a per-worker-polling design Start abandoned in favour of one fetchLoop; WithStaleJobThreshold's doc didn't say it has no effect on any first-party backend, since all five implement job.LeaseStore and route through reclaimExpiredLeases instead, which reclaims purely by lease expiry. - runner.go's commitOutputEntries pointed at a rollback rationale in commitOutputFile's doc that 99e34b3 deleted; inlined the actual rationale instead of the dangling reference.
…, and the kill ladder - exec.Request's doc and its Env field both said "nothing is inherited from the worker's environment," stated absolutely, twice, plus a third copy of the same claim in exec/subprocess/doc.go. buildEnv copies a PATH/HOME/TMPDIR allowlist from the worker's own environment before anything else — executor.go already states this correctly one package away. HOME in particular is what locates ~/.aws, which this rung's uid boundary (not environment exclusion) is what actually protects. - exec.OutputFile's doc said the worker "verifies the claim against what is actually on disk." It never reads Result.Outputs at all — commitOutputs is driven entirely by collectOutputEntries walking req.OutputDir. Rewrote to say the sandbox's claim is never consulted, not checked and rejected — a stronger guarantee than the old wording implied, and the one an auditor asking "is sandbox-reported metadata trusted anywhere" needs stated correctly. - exec/doc.go's leaf-package claim named id, scope, and the root dispatch package; the actual intra-repo imports are id and artifact, matching deps_test.go's own allowlist. Neither scope nor the root package is imported. - kill_unix_test.go and main_test.go both described a missing Setpgid as leaving the grandchild running while the leader still dies. Without Setpgid the child stays in the worker's own process group; terminate's pgid (the child's pid) names no real group, so both the SIGTERM send and the escalating SIGKILL come back ESRCH and NEITHER process is ever signalled — a materially larger blast radius than the comments credited. Also corrected main_test.go's description of envGroupKill's fixture order: it ignores SIGTERM before forking, not after (the safer order, closing the pre-fork SIGTERM race). - Two dangling cross-references: limits_unix_test.go pointed at TestSameUserIsRefusedByDefault's neighbors (uid tests) for rlimit portability reasoning that lives in exec/shim/rlimit_unix_test.go instead. rlimit_unix_test.go claimed to be unix-tagged "unlike internal_test.go," which is unix-tagged too, and claimed every case uses a negative value, when TestApplyOneUnverifiedResourceIsAFailure uses "12345" (safe because resourceOK is false, not because of sign). - kill_unix_test.go's TestKillLadderClassifiesACooperativeTimeoutCorrectly claimed to be the only test driving classify's timedOut-overrides- the-frame rule through a real process; exectest's conformance suite runs the same shape twice more (testDeadlineEnforcedCooperative, testDeadlineEnforcedSwallowedCancellation) against this rung. Also hardened TestKillLadderReapsAHelperAfterACooperativeLeaderExits's ESRCH assertion, which failed once under parallel-package load: the helper is a grandchild reparented after its leader dies, so a zombie that has already been killed but not yet reaped still answers syscall.Kill(pid, 0) successfully. Poll for up to 2s instead of checking once — the assertion still requires ESRCH, just tolerates the reap not being instantaneous. Ran 8x locally with no failures.
The five job.LeaseStore.ReclaimExpiredLeases backends used to disagree about limit <= 0: mongo returned nothing, postgres errored on a negative limit, sqlite treated negative as unlimited, and memory and redis treated both zero and negative as unlimited. We standardize on mongo's guard everywhere: a non-positive limit claims nothing and returns (nil, nil), checked before any query runs. That's exactly what DequeueOpts.Limit already does, so the two methods on the interface finally agree. This also clears out the dead capacity math in redis's allocation (max(limit, 0)) and rewrites the doc comment on job.LeaseStore.ReclaimExpiredLeases to state the unified contract instead of the old per-backend split. Every backend's lease_test.go now asserts a non-positive limit reclaims nothing, and that the job stays reclaimable by a later call with a positive limit.
requeueAfterLaunchFailure was the one terminal write that never went through updateJob. It called store.UpdateJob directly, so it carried no epoch predicate and applied unconditionally, stale worker_id and lease_epoch included. A worker whose lease had already moved on could still requeue a job a second worker was actively running, rolling the epoch backwards and leaving both attempts believing they owned it. Route it through r.updateJob like every other terminal write, and send job.ErrLeaseLost to abandonLostLease so a fenced-out attempt discards its write instead of stomping the real holder's row. Extended TestRunner_TerminalWrites_AbandonOnLeaseLost with a fourth case for this path; confirmed by mutation that it fails against the old unfenced call and passes against the fix. Also updated the comment in terminalFor that explained why a fence-lost commit error is deliberately kept off StatusLaunchFailed. The reason it gave (the unfenced write behind that status) no longer applies now that both paths are fenced identically, so the comment now says that plainly instead of describing a hazard that is already closed.
TestEngine_JobIsDispatchedToTheAddedExecutor checked rung.counts()'s reclaimed value before calling eng.Stop. That was correct while Pool.Start ran the Reclaim sweep synchronously, but 5b01c86 moved the sweep into a background goroutine that only Stop's own p.wg.Wait() joins. The assertion has been racing that goroutine ever since: failed 5/5 in isolation. Moved the check after eng.Stop returns, next to the closed assertion that was already correctly placed there. Same assertion, same bound of exactly 1, just read once the sweep is guaranteed to have finished. Ran it 5/5 in isolation and in the full package to confirm.
245aab6 closed this hole for postgres and sqlite, and only there. It was still open on both remaining persistent backends. A job sitting in state running at the instant a fleet upgrades has no lease_expires_at, every backend's ReclaimExpiredLeases requires that field to be non-null, and the pool stopped calling ReapStaleJobs for any store implementing job.LeaseStore. Dequeue claims only pending and retrying rows. Nothing looked at those jobs ever again and they held their slots forever. Mongo takes the backfill, in the shape 245aab6 established. It hangs off Store.Migrate rather than migrations.go, because the grove migration group in that file is not run from anywhere: extension.Start calls store.Migrate, and that method only ever created indexes. The filter tests lease_expires_at against null rather than $exists, since the collection genuinely holds both shapes for the same absent value (grove's insert path writes an explicit null, the driver's own encoder honors omitempty and drops the key), and plain null equality is the one test that matches both. That is also what makes it safe to re-run, since after a pass those rows have a non-null expiry and match nothing. The value is copied from heartbeat_at, then started_at, then a bound time.Time, which needs a pipeline update because $set alone cannot read another field of the same document. Redis takes a predicate instead. It has no migration mechanism to hang a backfill on, Migrate being a no-op, so reclamation itself carries the compatibility clause: a running job with no lease at all is adopted once its heartbeat_at, or started_at when it never beat, is older than a fixed 15 minute window. job.Lease.IsExpired is untouched and stays the only authority for the leased case. The staleness gate is the whole safety argument, not a detail. DequeueOpts.Grants() is false whenever LeaseUntil is zero, so a caller using job.Store directly without lease options holds a perfectly healthy running job with no lease. A null expiry by itself does not mean abandoned. A worker that is still heartbeating is never touched, and a row with neither timestamp is left alone because there is nothing to measure age against. The window is arbitrary and no operator can tune it, which is worth saying out loud rather than burying: ReclaimExpiredLeases carries no threshold, and widening that signature is a five backend change for a clause that stops mattering once a fleet finishes upgrading. Before leases these same rows were reaped at Config.StaleJobThreshold, 30 seconds by default, so 15 minutes is strictly less aggressive than what already shipped. Backfilled and adopted jobs alike reach the normal reclaim path with an expiry in the past, so a job an old pod is still running gets evicted and retried elsewhere. That is within at-least-once and matches what the SQL backends do. Redis is narrower on this point, since a live worker's heartbeat keeps it out of range, which a one-shot backfill cannot manage. Mutation verified on both backends. Removing the backfill strands all three mongo cases. Seeding a non-null value 24 hours in the future, which is the sqlite trap from 245aab6 in its mongo form, still strands the row that has no timestamps to copy, and only an assertion that ReclaimExpiredLeases collects the job catches it. Removing the redis clause strands both stale cases, and dropping the staleness gate reclaims all three healthy ones.
exec/shim/accessor.go claimed Path resolves for a declared input even though Ref does not. It doesn't: nothing populates req.Inputs or req.InputDir for an out-of-process attempt yet, so Open returns ErrUnbound for the same reason Ref returns false. The doc comment now says so and points at execution-isolation.mdx's Phase 3 note instead of contradicting it. worker/runner.go's abandonLostLease named the wrong sweeper for a losing attempt's outputs. They carry a link, so SweepOrphans (link-less only) never touches them; SweepEphemeral's owner-terminal path is what actually collects them once every linking owner has gone terminal.
Five requeue paths, four different ideas of what "back to pending" clears. ReclaimExpiredLeases is canonical: it clears WorkerID, StartedAt, HeartbeatAt, and LeaseExpiresAt. requeueRateLimited and requeueAfterLaunchFailure touched only State and RunAt; requeueUndispatched also cleared StartedAt but left the rest; reapStaleJobsLegacy cleared WorkerID/HeartbeatAt/StartedAt but not the lease fields. None of that is a correctness bug by itself, since ReclaimExpiredLeases only reclaims jobs in the running state and the next claim overwrites whatever a pending row still carries, but it leaves an operator staring at a pending job that looks assigned to a worker that walked away from it hours ago. clearJobAssignment is now the one place that decides what "no longer assigned" means, and every requeue path calls it before writing. Deliberately not touched: LeaseEpoch and EvictCount. Those are lease-eviction bookkeeping for a worker reclaiming someone else's job, and every caller here is a job's own current holder putting it back, not a reclamation. TestPoolRequeuesJobThatDoesNotFitLocally relied on StartedAt surviving requeueRateLimited (which requeueLocalMisfit reuses) as proof a claim happened. It doesn't survive anymore, so the test now watches RunAt get pushed forward instead and asserts WorkerID/StartedAt are cleared, which is closer to what the test actually wants to prove.
… StaleJobThreshold alone checkReaperMargin failed Build when StaleJobThreshold sat below twice the claim-to-first-heartbeat window, to stop the reaper reclaiming a job the fetcher still holds during a resource-admission stall. The problem: on every first-party backend, reapStaleJobs routes to reclaimExpiredLeases once the store implements job.LeaseStore, and that reclaims purely on lease expiry, resolved as LeaseTTL, then DefaultLeaseTTL, then StaleJobThreshold, then job.DefaultLeaseTTL. The check never looked at DefaultLeaseTTL. PollInterval=5s, HeartbeatInterval=5s, StaleJobThreshold=5m, DefaultLeaseTTL=12s used to pass Build with a real reclaim window of 12s against a required minimum of 20s. That's the exact double-execution the check exists to prevent, waved through by the check. checkReaperMargin now takes whether the store is lease-aware and polices the number that actually governs there: DefaultLeaseTTL falling back to StaleJobThreshold falling back to job.DefaultLeaseTTL on a lease-aware backend, StaleJobThreshold directly on one that isn't. Added the reproduction above as a table case, plus its mirror image (a non-lease-aware backend, where DefaultLeaseTTL genuinely doesn't matter).
Track D's lease reclaim increments EvictCount, never RetryCount, so a zombie holder and the worker it was fenced out for can both commit a link at the same (OwnerKind, OwnerID, Name, Attempt). CreateFenced closes the storage-key collision between them, but nothing stops two Link rows with different ArtifactIDs at that identical tuple, and FindLinkByName's only tie-break was Attempt itself. On memory, whose links live in an append-only slice, resolution went to the zombie 5 times out of 5: the first insert always won a tie, and reclaim always inserts before the winner's own commit. Every backend now breaks a tie by CreatedAt descending: attempt DESC, created_at DESC on postgres and sqlite, the same pair on mongo's sort, and a CreatedAt.After comparison on memory and redis's map-walk. Redis can't actually produce this collision (LinkArtifact keys its hash by name+attempt, so a second write overwrites rather than coexisting), but the tie-break is there anyway so it agrees with the other four if that storage shape ever changes. This is the cheap half of the fix. Carrying the fence token on Link itself, so the zombie's write is rejected outright instead of merely outrun, needs a schema change across five backends and stays open; the doc comment on artifact.Store.FindLinkByName says so. Added FindLinkByNameTieBreaksOnLatestWrite to the shared artifacttest suite so all five backends are held to the same tie-break, verified against postgres, mongo, and redis containers plus memory and sqlite.
worker/pool.go's fetchLoop always sends Budget, CustomKeys, WorkerID, and LeaseUntil together in a single DequeueOpts. There's no path that grants a lease without also carrying whatever budget the worker has. The shared conformance suite tested the two halves apart: dequeue.go never touches LeaseUntil or WorkerID, lease.go never touches Budget. A backend could pass both suites while still getting the composed shape wrong, for instance by granting the lease before the fit predicate ran. DequeueComposesBudgetAndLeaseGrant enqueues a job that fits a memory budget and one that doesn't, dequeues both with a budget and a lease grant in the same call, and checks the fit predicate still filtered the oversized job while the surviving one got a real lease (epoch, expiry, worker). Runs on all five backends through RunLeaseSuite; verified against postgres, mongo, and redis containers plus memory and sqlite.
job.WithResourceLimits documents ResourceLimits as the enforcement ceiling. It gets resolved at enqueue and persisted with columns and migrations across five backends, and nothing anywhere read it back. Two jobs declaring 8 GiB and 256 MiB got the identical RLIMIT_AS, because the subprocess rung's rlimits came from a deployment-wide subprocess.WithRlimits only. exec.Request now carries ResourceLimits, a resource.Set built from the job's resolved ceiling in Runner.request. exec is allowed to import resource under the leaf constraint exec/deps_test.go polices, since resource is a leaf itself and imports neither job nor exec. exec/subprocess.Executor.buildEnv maps resource.Memory to RLIMIT_AS: a job's own limit wins when it declares one, and the deployment-wide AddressSpace is the fallback for the overwhelming majority of jobs that declare nothing. resource.CPU has no clean rlimit equivalent (RLIMIT_CPU caps total CPU time, not an instantaneous millicore share), so it stays unmapped rather than getting invented semantics; every other Rlimits field (NoFile, NProc, FSize) has no per-job counterpart in resource.Set at all and stays deployment-wide only. Not built: the full SpecFrom(ctx) contract from the resource-model spec. This wires the concrete path that makes the shipped option honest without building the larger design.
Two issues arrived with the pre-lease adoption tests and put golangci-lint run ./... at exit 1 for the whole branch. The mongo one is a shadowed err. The redis one is prealloc firing on a table built as a literal and then appended to; rather than preallocate, the appended case now folds into the literal behind a withoutTimes helper, matching the withHeartbeat helper already in that test. That removes the append the linter was objecting to instead of working around it, and the table reads as one list again.
TestConcurrentStageAndReclaim was failing about one full-suite run in three with a staged path whose file was already gone. It is not a flaky test. A lease really was failing to pin a file, and the test found the bug it was written for. evictOne dropped the victim from the entry table under the table lock, then unlinked the file after letting the lock go. In that gap a download of the same hash could reach promote, stat the victim's file, find it still on disk, and adopt it instead of renaming its own copy into place. It then registered an entry for that path and handed a caller a live lease. The eviction's unlink landed a moment later and the caller's file was gone. Worse than the one bad lease: the poisoned entry stayed in the table pointing at nothing, so every later stager of that artifact was handed the same dead path until the entry was evicted again. That is why two different stagers at two different rounds both lost the same blake3 path. The fix is to make both transitions atomic against each other. Eviction now unlinks through a callback that runs inside evictLRU, and the download side goes through a new entryTable.publish that does the rename and the registration in one critical section. A file exists at a hash's path if and only if the table holds an entry for that hash, and now both edges of that invariant are taken under the same lock. Releasing the victim's hold stays outside, because that takes the manager's lock and the two are still never held together. Evidence, on a 16-core box running 16 copies of the race-enabled test binary at -count=100. Before: 19 failing runs out of 1600, 32 assertions. After: 0 out of 1600. The new internal tests pin the mutual exclusion directly, since the concurrency test only catches this about one run in a hundred even under that much pressure; deleting the remove call from inside evictLRU's critical section makes them fail.
…ckfill 698a7eb gave mongo a migration backfill and redis a reclaim predicate, because redis has no migration mechanism to hang a backfill on. Running both approaches side by side made it obvious the predicate is simply the better one, and that the backfill was solving a smaller problem than the one that actually exists. A backfill only sees rows that exist at the instant it runs. That leaves two gaps. During a rolling upgrade the old pods keep claiming jobs after the first new pod has migrated, and every one of those is stranded exactly as before. Worse, this was never only a migration artifact: DequeueOpts.Grants() is false whenever LeaseUntil is zero, so any caller claiming through job.Store without lease options writes a running row with no lease at all, at any time, forever. The conformance suite covers that call. If such a worker dies the row was unreclaimable on mongo, postgres and sqlite, with or without any backfill. The backfill also seeds an expiry in the past, which hands the row to the next sweep. For a job an old pod is still running that is an eviction, not a recovery, because an old binary's heartbeats do not push an expiry it does not know about. So all four persistent backends now carry the same reclaim rule. The first branch is unchanged and job.Lease.IsExpired remains its only authority: a lease was granted and has lapsed. The second adopts a running job with no lease once it has been silent for job.UnleasedReclaimGrace, measured on heartbeat_at and falling back to started_at for a worker that died before its first beat. A row with neither timestamp is never adopted, since there is nothing to measure age against and guessing means guessing against a running job. The two backfills are gone, including the one 245aab6 added. Removing it from migration 008 is safe either way: a deployment that already ran it keeps those expiries, and one that has not gets the same rows adopted by reclamation instead. The grace constant lives in job/lease.go next to DefaultLeaseTTL so four backends read one value with one rationale. It is arbitrary and no operator can tune it, which the comment says outright rather than burying: ReclaimExpiredLeases carries no threshold and widening that signature would change all five backends. Before leases these rows were reaped at Config.StaleJobThreshold, 30 seconds by default, so 15 minutes is strictly less aggressive than what already shipped. The memory backend is deliberately left out. It is in-process and loses every row on restart, so it has no upgrade to survive. Within one process lifetime an abandoned unleased claim is still invisible there, which is worth knowing before anyone treats memory as a behavioural reference for the others. Mutation verified per backend, and the negative cases turn out to matter more than the positive ones. Dropping the staleness gate so a null expiry alone reclaims steals all three healthy rows on every backend, which is the result that justifies gating on silence at all. Dropping the whole clause strands the silent ones again. One mutation is worth repeating elsewhere. Formatting sqlite's cutoff as ISO-8601 text rather than binding a time.Time still fails, but it fails in the opposite direction from 245aab6: there the comparison ran the way that made a formatted value match nothing, so the damage was jobs staying stranded, no worse than having no backfill. This predicate compares the other way, so a formatted value sorts above every driver-written timestamp and matches everything, reclaiming live jobs from healthy workers. The same mistake fails open here rather than closed, and the comment at the bind says so. Full suite green on all five backends, including the integration-tagged migration tests, which the earlier run had missed.
f256bf8 left this backend out on the grounds that it has no upgrade to survive, which is true and covers only half of why the rule exists. Memory loses every row on restart, so no pre-lease build can hand it a running job with no expiry. But DequeueOpts.Grants() is false whenever LeaseUntil is zero, so a caller claiming through job.Store without lease options writes an unleased running job here exactly as it does everywhere else, and the conformance suite covers that call. Abandon one and nothing reclaimed it for the life of the process. That gap mattered more than its blast radius suggests. This store is the reference the SQL backends are checked against, as its own DequeueJobs comment says, so leaving it as the one backend that strands a row every other backend recovers makes it a misleading reference rather than a convenient one. The predicate is the same one the other four apply, reading the same job.UnleasedReclaimGrace, with job.Lease.IsExpired still the only authority for the leased case. Two comments went stale the moment the behaviour changed and are corrected here rather than left to mislead. DequeueJobs claimed an unleased running job "would be invisible to it rather than vulnerable to it", which was the bug stated as a feature. DequeueOpts.LeaseUntil claimed a half-granted job "can never be reclaimed", which is no longer true: it is recovered, but only by the coarse minutes-long fallback rather than the per-job TTL a lease buys. That distinction is the actual argument for granting inside the claim, so the comment now makes it that way instead of resting on a claim that has stopped holding. Mutation verified. Removing the clause strands both silent cases; removing the staleness gate steals all three healthy ones. Full suite green across all five backends, including the integration-tagged tests.
retryJob reset state, retry count, error and timestamps, then wrote the row back with UpdateJob. UpdateJob writes the whole row on every backend, lease columns included, so the retried job carried the LeaseExpiresAt of the run that had just failed back into pending. Nothing notices while it sits there. It goes wrong at the next claim that grants no lease, which is a supported call: DequeueOpts.Grants() is false whenever LeaseUntil is zero, and such a claim writes state and worker but never touches the expiry. The job therefore enters running already holding a lapsed lease, and the next reclaim sweep takes it straight back. Claimed, reclaimed, claimed again, with EvictCount climbing on every pass and the job never once running to completion. A retry through the API could put a job into a loop it could not leave. The reset now lives on job.Job as ClearOwnership, rather than as four more lines in the handler, because the rule generalises: any path returning a job to pending from outside the lease machinery has to drop the worker and lease fields, and the reason it matters is long enough to be worth writing down once. LeaseEpoch is deliberately not touched. It is a fencing token that must never move backwards, and there is nothing to fence, since a job on this path is not running and has no holder to invalidate. ReclaimExpiredLeases increments it because it is taking a job away from a live holder, which is a different situation. Reproduced end to end against store/memory rather than asserted on the struct, since the failure is a three-step interaction (requeue, claim without a grant, sweep) that no single-field check would have caught. The first subtest pins that the stale expiry really does reach running and get reclaimed, so the reproduction cannot quietly rot into passing for the wrong reason. Mutation checked: leaving LeaseExpiresAt stale fails with "a freshly claimed job was reclaimed". api has no test harness of its own, which is why the handler itself is still uncovered. Moving the logic somewhere testable was the point.
…dated Both described the heartbeat sweep that leases replaced, so anyone reading them to work out when a job comes back got the wrong mechanism and the wrong knob. reaperLoop said it reaps "stale jobs whose heartbeat has expired". That is the fallback path. For a store implementing job.LeaseStore, which is all five built-in backends, reapStaleJobs routes to lease reclamation instead, and the heartbeat matters there only because renewing the lease happens to write it. WithStaleJobThreshold said it sets how long without a heartbeat before a job is reaped. On those same five backends it no longer sets that window at all: the window is the per-job lease TTL, from WithLeaseTTL or the pool default. What the option still does is set how often the reaper looks and whether it runs at all, which is the part an operator tuning it needs. The old meaning does still hold for a store that is not a LeaseStore, so both are stated rather than one being swapped for the other. Also drops postgres' second job.LeaseStore assertion. store.go already asserts every subsystem interface in one block, and mongo and sqlite both carry a comment at this spot pointing there instead of repeating it; postgres now does the same.
dlq.Entry stored a job's identity, payload and retry budget and nothing about how the job actually ran. Replay rebuilds a job from the entry and calls EnqueueJob directly rather than going back through the engine, so nothing re-derives the rest on the way out: every field the entry did not carry, the replayed job silently took a default for. LeaseTTL is the one that turns this from untidy into broken. A job declaring a six-hour lease TTL, replayed, came back on the pool default, which is measured in seconds. Its lease then lapses mid-run, reclamation takes it back, it restarts, and it does that forever without finishing. That is precisely the failure per-job lease TTLs exist to prevent, so the DLQ was the one path that could reintroduce it. The rest were dropped the same way and matter for the same reason. Timeout meant a long job was killed early. Priority meant it lost its place. Resources, ResourceLimits and ResourceClass meant it looked free to schedule and could be claimed by a worker with no room for it. ArtifactBindings meant a handler declaring inputs got none, and InputBytes/PrimaryInputHash are derived from those bindings by the engine at enqueue, which Replay does not run, so they are carried rather than recomputed. Values are copied from the failed job rather than looked up from the definition by name. The definition is only half the answer: an enqueue site can override any of these, and a definition can be edited or removed between the failure and the replay. What is stored is what actually ran. A single JSON snapshot column was considered instead of nine fields, since it would carry future job fields for free. It was rejected because to be safe it would still need an explicit allowlist on restore -- otherwise it reinstates state, worker_id and the lease columns, which is the livelock 9a8a124 just removed from the retry path -- and with an allowlist the free-carrying property is gone. Explicit fields cannot express the bug at all: dlq.Entry has nowhere to put a worker id. Mongo and redis are schemaless and needed only struct fields. Postgres and sqlite get one batched migration each, following the shape their existing lease and resource migrations established: postgres batches its ALTER under a lock_timeout, sqlite guards every ADD COLUMN with pragma_table_info since it has no ADD COLUMN IF NOT EXISTS and grove runs Up outside a transaction. Both store the resource sets with the same codec the job tables use rather than as scalar columns; nothing queries a DLQ row by resource requirement, so the scalar split dequeue's fit predicate needs buys nothing here. Tested as a new conformance suite in store/storetest/dlq.go, run against all five backends, because "one backend silently drops one column" is the failure this has to catch and nothing at the service layer can see it. It lives in its own file rather than in lease.go, which another session is editing. Every value in the fixture is non-zero and distinct so a mapper that drops or crosses fields cannot pass by accident, and a second case pins that a job declaring no resources reads back with none rather than with an empty set, which is a real distinction on the SQL backends. The suite earned itself immediately: it caught a stray escape in the postgres migration that made the ALTER a syntax error on a real container. Mutation checked at all three layers independently, since each can drop a field on its own: Push not capturing LeaseTTL, Replay not restoring it, and the sqlite mapper not persisting it all fail with the TTL assertion. Not addressed: Replay still does not restore the job's own retry configuration beyond MaxRetries, and a replayed job is a new job by design, so anything keyed on the original job ID does not follow it.
…rom its limit Clears the deferred list the lease phase left behind. Two of the five were real, three were documentation, and one of the "real" two was a latent flake rather than the comment it was filed as. SQLite retried SQLITE_BUSY on a flat 1ms delay. SQLite takes one write lock for the whole database, so of N writers that collide exactly one wins and the other N-1 sleep the identical interval and wake together to collide again. The loser set stays in lockstep until it drains one at a time, which is the worst shape a backoff can have, and it now covers the claim path as well as the lease writes. The delay is jittered across half to one and a half times the base, centred on the old constant rather than added to it, so maxLeaseBusyRetries still takes about as long as it used to and this cannot quietly turn a fast failure into a slow one. Both properties are tested and mutation checked: a flat delay fails on distinct-value count, an added rather than centred jitter fails the bound. testReclaimIsExclusiveUnderConcurrency passed jobCount as its reclaim limit, which coupled the assertion to the fixture size. Reclamation is not queue-scoped and newStore may hand the case a store other cases have already left expired jobs in, so four reclaimers could fill their quotas with those, leave some of the case's own jobs unclaimed, and fail the "never claimed" branch for a reason unrelated to exclusivity. Filed as a comment to add; it reads as a flake waiting to happen, so the limit is now large enough never to bind. What the case measures is double-claiming, not throughput. Reclaim ordering genuinely differs and is now documented on the interface rather than unified. Postgres, SQLite and Mongo take longest-expired first because they already read through an index on the expiry; memory iterates a map and redis walks an unordered set. Redis is the reason this is written down instead of fixed: it can stop as soon as it has claimed `limit` jobs, and ordering would force every call to read the whole job-id set first, turning a bounded scan into a full one on exactly the deployments least able to afford it. Nothing starves either way, since each pass removes what it takes from the eligible set. The SQLite package doc now covers write concurrency, because the two knobs that would normally handle it are both out of reach: grove's driver sets no busy_timeout, so a losing writer fails immediately instead of waiting, and it does not expose the *sql.DB, so this package cannot cap the pool at one connection. The Go-side retry is a mitigation, not a substitute, and the doc says what to do when it is not enough. The dead exported surface is documented, NOT removed. This module is v1.6.0, so deleting ErrLeaseNotSupported, EvictReason or the unpopulated Lease fields is a breaking change, and none of them is a mistake that wants deprecating: they are surface that was declared ahead of the code that would use it. Each now says what it is for and that nothing currently returns, persists or populates it, which is the part that would otherwise mislead. Lease in particular is worth stating plainly, since backends build one solely to call IsExpired and leave the other three fields zero. Two comments that the reclaim change had already invalidated are corrected while here. DequeueJobs claimed a row left running with no expiry is "invisible to ReclaimExpiredLeases", which stopped being true when reclaim started adopting unleased rows; it now says such a row waits out the far coarser UnleasedReclaimGrace instead of the TTL it was meant to get, which is still the argument for granting inside the claim. IsExpired's doc gets the counterpart, since read alone it now looks like it contradicts UnleasedReclaimGrace: reclamation does eventually take a never-leased job, but never through that function, and the two questions are deliberately kept apart.
…se suite Five backends each carried their own copy of the same test, written when b944b1d unified the limit contract and deliberately left out of the shared suite because another session was editing that file. It has since landed its work, so the copies collapse into one conformance case: 236 lines out, 70 in, and the contract now applies to any backend that runs the suite rather than only to the five in this repository. The per-backend doc comments were the part worth keeping, so they are merged into the shared case rather than deleted with the code. Each backend broke differently before the guard, and two of them broke in opposite directions: a negative limit reached Postgres as `LIMIT $1` and the server rejected the whole statement with SQLSTATE 2201W, while SQLite defines a negative LIMIT as "no limit" and reclaimed the entire table. Mongo already returned early, and memory and redis read non-positive as unlimited. That spread is the argument for the case living in the suite, and it now reads that way instead of being split across five files that never mention each other. Mutation checked on the two that fail in opposite directions, which is the pair a single shared case has to cover: removing the sqlite guard fails with "reclaimed 1 jobs, want 0", removing the postgres guard fails with "LIMIT must not be negative". Also fixes a container flake I introduced in 6579491's neighbour commit. TestReclaimAdoptsRunningJobsWithoutLease called setupTestStore inside its table loop, so five postgres containers started and stopped in sequence, and under a full-suite run one of them failed to publish its port. It now starts one container for every case, which is both five times fewer chances to fail and about four times faster. Sharing the database means the result set is no longer this case's alone, so the assertions move from "reclaimed exactly this one job" to membership through storetest.Contains, which is the same correction made to testReclaimIsExclusiveUnderConcurrency in 93840e8 and for the same reason: reclamation is not queue-scoped, so asserting on the shape of the whole result couples a case to whatever its neighbours left behind.
Engine.Stop guards its executor close with a sync.Once and documents that the rest of Stop tolerates a second call because the subsystems check their own flags. Checking that claim rather than trusting it turned up one place it did not hold. Dispatcher.Stop gated only the pool call, and only through the started flag, which Start sets and nothing clears. EmitShutdown and the store Close ran on every call. So a second Stop emitted a second shutdown event to every extension and closed the store again. Neither is hypothetical: Engine.Stop reaches this, and a service shutting down from both a signal handler and a deferred cleanup calls Engine.Stop twice. The built-in backends survive it only because their Close is a documented no-op; a custom Storer promises nothing of the kind, and neither does an extension asked to release its resources twice. It was also a data race. Two goroutines calling Stop both read started before either wrote anything, and the counting test reproduces seven closes out of eight concurrent calls. Fixed with a sync.Once on the Dispatcher, matching cron.Scheduler, which already guards its own Stop that way. A flag would not do: the two callers that make this reachable are usually different goroutines, which is the case an unsynchronised bool cannot separate. A second call returns nil rather than repeating the first call's error, matching Pool.Stop. The engine comment is corrected to match what is now true, including that a second Stop returns nil for exactly this reason. Its narrow scope is left alone and the reasoning written down: every other step owns its idempotence at the layer that knows what repeating it costs, and widening the guard would move that decision to the wrong place and hide from a reader that the subsystems already handle it. Verified with counting fakes over the existing internal interfaces, both sequentially and under eight concurrent callers, the latter also under -race.
BREAKING CHANGE: job.ErrLeaseNotSupported, job.EvictReason and its two constants, and the JobID, WorkerID and Epoch fields of job.Lease are gone. Nothing in this module referenced any of them, but the module is tagged v1.6.0, so anything outside it that names one stops compiling. Under Go's module rules this is a v2 change; it is being taken on v1 as a deliberate call, on the grounds that surface nobody uses is cheaper to remove now than after someone starts. Every one of these was declared ahead of code that never arrived, and each had grown a comment explaining that nothing used it, which is a poor substitute for not having it. ErrLeaseNotSupported was never returned. A pool given a store without the capability degrades to the heartbeat reaper and logs, because refusing to start over a missing optional capability would be worse than running without it, so there was no call site left for a sentinel. EvictReason named the two ways a job loses its worker, but no column stores which one happened. Job.EvictCount records that an eviction occurred and is what retry-budget decisions actually read. Recording the reason would mean another column on five backends, and the constants were not what stood in the way of doing it. job.Lease kept a JobID, a WorkerID and an Epoch that no backend ever set: all five build a Lease solely to call IsExpired and read the holder and the fencing token off the job row, where they live as Job.WorkerID and Job.LeaseEpoch. Three of the four fields were decoration, and the risk is not the space they took but a caller reading Epoch off a Lease a backend returned and getting a zero that looks like a real epoch. What is left is the part that earns its place: a single ExpiresAt and IsExpired, which is the one authority on whether a lease has lapsed and the home of the rule that a zero expiry means "never leased" rather than "expired". The epoch documentation is not lost, since Job.LeaseEpoch already carried the fuller version; the one line it lacked, about a worker resuming from a long GC pause, moves there.
Picks up the CI and dependency work main landed while this branch ran, so a pull request against main is reviewed and built on current config rather than on what this branch forked from. go.mod and go.sum conflicted on dependency versions only, no code. Resolved by taking main's newer versions across the board (grove v1.5.9 to v1.6.0 and its five drivers, relay, vessel, and the indirect graph) while keeping the requirements only this branch has, then regenerating with go mod tidy. That tidy moved klauspost/cpuid from v2.0.12 to v2.4.0, which reads CPU features through syscalls where older versions used CPUID instructions alone. It reaches exec/shim through blake3's SIMD dispatch, so the sandbox closure gained golang.org/x/sys/unix and TestShimLinksNoInfrastructure failed, exactly as it is built to. The allowlist gains golang.org/x/sys rather than the test being loosened: raw syscall bindings are the furthest thing from the infrastructure clients that list exists to keep out, and the existing prefix rule could not cover it because x/sys is its own module rather than a subpackage of something already allowed. Full suite green on all five backends, including the integration-tagged tests, which matters more than usual for a grove bump: the sqlite reclaim predicate is a string comparison against that driver's own rendering of a time.Time, so a change in how it formats one would have broken reclamation silently. The lease and DLQ suites assert reclaim collects the row, so they would have caught it.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
gosec found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is the long-run durability and execution isolation work, four strands of it, landing together because they grew into each other while they were built. It is large: 179 commits, 267 files, and it will not review in one sitting.
Read it strand by strand rather than commit by commit.
What is in here
Execution isolation. Handlers can now run in a separate process. There is a subprocess rung with rlimits, a dedicated UID and a kill ladder, a shim child entrypoint with a directory-backed artifact backend, and a length-prefixed wire codec between them. The shim's dependency closure is guarded by a test, since a sandbox process that links a database driver is not a sandbox.
Resource-aware scheduling. Jobs declare what they need and workers admit them against real capacity. All five backends widened their dequeue to the resource-aware contract, and the cache admits staged bytes against the same shared ledger the engine uses, so the two cannot disagree about what is free.
Job leases. A worker holds a lease on a running job, renews it, and stops when it loses it. The grant is folded into DequeueJobs rather than written afterwards, because a claim and a grant as two writes leaves a window where a crash strands the job. Reclamation is fenced on an epoch, so a worker returning from a long pause cannot write over the job someone else now owns.
The artifact plane. Job inputs and outputs move as artifacts with content hashing, scoped ephemeral keys, and a sweep for what gets orphaned.
Before you merge
There is a blocker you have to deal with first.
go.modcarriesreplace github.com/xraph/trove => ../trove, which came in on this branch and is not on main. CI has no../trove, so it will fail, and it must not reach main either way. Somebody needs to drop it or point trove at a published version.Two commits are breaking and marked with
!.feat(job,store)!changed the dequeue signature to carry the lease grant.refactor(job)!removedjob.ErrLeaseNotSupported,job.EvictReasonand its constants, and the JobID, WorkerID and Epoch fields ofjob.Lease, none of which anything referenced. The module is tagged v1.6.0, so under Go's rules that second one belongs behind a /v2 path and is being taken on v1 deliberately. It wants a release note, and there is no CHANGELOG here to put one in.Postgres and SQLite both gain migrations. They add columns to
dispatch_jobsanddispatch_dlqand are written to survive running against a fleet that is still working: Postgres batches its ALTER under a lock timeout and builds indexes CONCURRENTLY, SQLite guards every ADD COLUMN because it has no IF NOT EXISTS and grove runs migrations outside a transaction. Mongo and Redis are schemaless and need neither.How it was checked
The whole suite passes on all five backends, including the integration-tagged tests, against real containers rather than fakes.
The lease and DLQ conformance suites are worth knowing about if you are reviewing the store layer. They assert that reclamation actually collects a row and that a replayed job comes back with the lease TTL it had, rather than that some column went non-null. That distinction caught two bugs that a weaker assertion passed straight through, one of them a backfill that wrote every recovered row into the future and left it permanently unreclaimable.
Main was merged in rather than rebased, so review history stays intact. That merge bumped grove to v1.6.0, which matters more than a version bump usually does here: the SQLite reclaim predicate is a string comparison against that driver's own rendering of a
time.Time, so a change in how it formats one would have broken reclamation quietly. The suite covers it.