kyleandClaude Opus 5.5
a452aa16a1
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>
30 lines
827 B
Go
30 lines
827 B
Go
package api
|
|
|
|
// P6 owner review: with batch off (disabled, or the store did not open) every batch endpoint
|
|
// answers 503; none of them panics on the nil runner.
|
|
|
|
import (
|
|
"net/http"
|
|
"testing"
|
|
)
|
|
|
|
func TestBatchDisabled(t *testing.T) {
|
|
s, keys := newBatchServer(t, "test")
|
|
s.Batch.Close()
|
|
s.Batch = nil
|
|
for _, c := range []struct{ method, path string }{
|
|
{http.MethodPost, "/v1/batch"},
|
|
{http.MethodGet, "/v1/batch/x"},
|
|
{http.MethodGet, "/v1/batch/x/0"},
|
|
{http.MethodDelete, "/v1/batch/x"},
|
|
} {
|
|
var body any
|
|
if c.method == http.MethodPost {
|
|
body = map[string]any{"targets": []any{map[string]any{"url": "http://127.0.0.1/"}}}
|
|
}
|
|
if rec := batchReq(t, s, c.method, c.path, keys["test"], body); rec.Code != 503 {
|
|
t.Errorf("%s %s: %d %s, want 503", c.method, c.path, rec.Code, rec.Body.String())
|
|
}
|
|
}
|
|
}
|