Owner review of p6 (Ornith tasks 01-09), with an independent reviewer pass whose findings I
confirmed against the code. Every fix has a regression test that fails on the old code.
internal/batch
- Close let the workers keep draining every live job under the cancelled context, failing
every target; they now stop at once, store nothing on a failure during shutdown, and leave
the job live for MarkInFlightFailed("restart").
- A job cancelled while its in-flight target came back busy stayed in the rotation forever
(depth gauge stuck, job held in memory); it now leaves. The task 03 design had this gap: mine.
- Submit's idempotency lookup, caps and create ran unlocked: 20 concurrent submits by one
caller made 8 jobs. Serialised with submitMu.
- A failed target insert left a queued job no worker would run (holding the caller's cap);
it is marked failed.
- A second concurrent cancel returned true and counted twice.
- Config: zero OR negative values take the defaults (negative Workers panicked the
WaitGroup; a negative TTL or byte budget would sweep or evict everything).
- Retry timing used the pre-fetch time (a 5 s busy answer made RetryWait moot).
- The rotation cursor is kept within [0, len(live)] (it was absolute and drifted on removal).
- The unused in-memory callers map is gone (Submit racing Close wrote to a nil map).
internal/dcache
- PutBatchTargets, SweepBatch and eviction are transactional; the sweep never removes a
queued or running job.
internal/api
- Every /v1/batch endpoint answers 503 when batch is off; POST panicked on the nil runner.
- GET /{id}/{idx} bounds idx by the rows actually returned.
- batchFetch copies a *fetch.Error instead of mutating one the PDF pool may share.
internal/config
- ParseSize no longer overflows to a negative size.
cmd/sift
- sift batch treated 202 Accepted (submit) as an error and exited 1 with no message.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Start cfg.Workers goroutines in New that drain one rotation of live jobs in
submit order, each taking the next job with a pending target and none in flight,
so targets alternate across jobs and each job stays serial. One target per fetch
under the Close-cancelled context: panic recovers into ErrInternal (never
echoed), ErrRetryLater retries after RetryWait (bounded by RetryFor), otherwise
the target is stored done/failed and the hooks fire. Terminal status is set once
per job; cancelling a running job stores the in-flight target without overwriting
cancelled and leaves later targets pending.
Co-Authored-By: ornith-1.5-35b-a3b <ornith-1.5-35b-a3b@llama.cpp.invalid>
New package internal/batch holding the runner core with no workers yet (task 03
adds them; until then every job stays queued, so a queued job counts as live for
the caps). batch.go defines the public surface: Options/Target/Result/TargetError,
FetchFunc, Job/TargetStatus, Hooks and Config with its defaults (TTL 24h, 32
targets/job, 1 job/caller, 8 total, worker clamped to 2, 64 MiB, 2 s / 10 m retry).
Submit runs in the given order: empty → ErrNoTargets; idempotency via hex SHA-256
of `c.Name + NUL + json.Marshal(targets)` (a live BatchJobByIDem hit returns the id
reused, touching no caps); then TargetsPerJob / JobsPerCaller / JobsTotal; then it
stores the job queued (random 16-byte hex id, CreatedAt = Lease = now) and its
targets (pending, Options = json of t.Options) before returning. The submitting
caller is held in memory keyed by job id, never persisted (jobs never resume after
a restart). Status maps the rows and decodes Result from the result column without
writing; Touch moves the lease; Cancel sets a queued job cancelled at once and
leaves its targets pending; Close is idempotent. Store errors are Warn-logged with
the job id, never a URL or host.
Co-Authored-By: ornith-1.5-35b-a3b <ornith-1.5-35b-a3b@llama.cpp.invalid>
kyleandClaude Opus 5.5
