diff --git a/README.md b/README.md index 5ffb2c4..b11d079 100644 --- a/README.md +++ b/README.md @@ -34,9 +34,10 @@ sift serve [-listen addr] [-config /etc/sift/config.toml] [-keys /var/lib/sift/ sift fetch [--question q] [--mode clean|raw|digest] [--jq filter] [--pages a-b] [--grep re] [--no-context] [--recover] [--json] sift extract [--pages a-b] [--grep re] [--question q] [--mode clean|digest] [--json] -sift search "" [--k n] [--json] -sift keys add --caller name [--workspace ws] [--out file] | list | revoke --caller name -sift version + sift search "" [--k n] [--json] + sift batch submit|status|get|cancel + sift keys add --caller name [--workspace ws] [--out file] | list | revoke --caller name + sift version ``` `sift fetch`, `sift extract` and `sift search` are clients: they read `url` and `key_file` from @@ -46,6 +47,42 @@ answer exactly as `sift fetch` does for a PDF URL — `--pages`, `--grep` and `- same, and a file over 100 MiB is refused before the upload. The agent-facing guide is [`skill/SKILL.md`](skill/SKILL.md). +## Batch + +`sift batch` runs several fetches as one job, so an agent can read a handful of URLs later instead +of a loop of `sift fetch`. The four endpoints: + +| Endpoint | What | +|---|---| +| `POST /v1/batch` | `{"targets":[…]}` → `202 {"success","job_id","status","targets","reused"}` | +| `GET /v1/batch/{id}` | summary; `?results=1` inlines each done target's `/v1/fetch` envelope | +| `GET /v1/batch/{id}/{idx}` | one target's result, as `sift fetch --json` | +| `DELETE /v1/batch/{id}` | cancel a queued job → `200 {"success","status"}` | + +`sift batch submit ` reads a JSON object with a non-empty `targets` array locally, +or a bare array of targets or URL strings (each string becomes `{"url": s}`), then posts it. `status` +prints the summary (or the body with `?results=1`); `get ` prints one result; `cancel` +stops a queued job. + +A job runs **serially** (target `i+1` never starts before `i` is stored) with **one worker by +default** (see `[batch]` config) and the runner rotates jobs **round-robin** while several are live. +Caps: **32 targets** per job, **1 live job per caller**, **8 live jobs** in all. The posted body is +capped at **256 KiB**. A target that is blocked, times out, or errors is recorded and the job +continues; a job whose targets are all done is `done`, all failed `failed`, mixed `partial`. + +**Idempotency**: the same list from the same caller returns the same job while it is live or already +has results (`reused: true`), so a retry does not start a second job. Each job carries a **lease** +that a status read renews; a job whose lease has expired for more than the **24 h TTL** is swept +along with its targets. On restart, jobs in flight when the server died are marked failed so a retry +re-submits them. + +A `status` read is a **summary** (per-target status, never the content); `?results=1` inlines the +full envelope for each done target, so it can be large — prefer `get ` for one result. +Restricted callers see the same endpoints; their results are **redacted** the same way a restricted +`sift fetch` is. Batch counters are `sift_batch_jobs_total{outcome}`, +`sift_batch_targets_total{outcome}`, `sift_batch_queue_depth`, `sift_batch_targets_inflight`, +`sift_batch_job_seconds`, `sift_batch_target_seconds` and `sift_batch_result_bytes`. + ## Configuration `/etc/sift/config.toml` (`$SIFT_CONFIG`); every key is shown in diff --git a/cmd/sift/batch_review_test.go b/cmd/sift/batch_review_test.go new file mode 100644 index 0000000..aeb8a9a --- /dev/null +++ b/cmd/sift/batch_review_test.go @@ -0,0 +1,52 @@ +package main + +// P6 owner review: the batch CLI treats every 2xx as success. POST /v1/batch answers 202, and +// batchDo exited 1 on it with an empty message. + +import ( + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestBatchDoAccepts202(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Authorization") != "Bearer k" { + w.WriteHeader(401) + _, _ = io.WriteString(w, `{"error":"unauthorized"}`) + return + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusAccepted) + _, _ = io.WriteString(w, `{"success":true,"job_id":"j1","status":"queued","targets":1,"reused":false}`) + })) + defer srv.Close() + dir := t.TempDir() + keyPath := filepath.Join(dir, "key") + if err := os.WriteFile(keyPath, []byte("k\n"), 0o600); err != nil { + t.Fatal(err) + } + cfg := filepath.Join(dir, "client.toml") + if err := os.WriteFile(cfg, []byte("url = \""+srv.URL+"\"\nkey_file = \""+keyPath+"\"\n"), 0o600); err != nil { + t.Fatal(err) + } + t.Setenv("SIFT_CLIENT", cfg) + + r, w, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + old := os.Stdout + os.Stdout = w + batchDo(http.MethodPost, "/v1/batch", strings.NewReader(`{"targets":[{"url":"https://example.com/"}]}`)) + os.Stdout = old + _ = w.Close() + out, _ := io.ReadAll(r) + if !strings.Contains(string(out), `"job_id": "j1"`) { + t.Fatalf("stdout %q, want the 202 body printed", out) + } +} diff --git a/cmd/sift/batch_test.go b/cmd/sift/batch_test.go new file mode 100644 index 0000000..7315e11 --- /dev/null +++ b/cmd/sift/batch_test.go @@ -0,0 +1,46 @@ +package main + +import ( + "slices" + "testing" +) + +func TestParseBatchArgs(t *testing.T) { + cases := []struct { + args []string + verb string + rest []string + ok bool + }{ + {[]string{"submit", "targets.json"}, "submit", []string{"targets.json"}, true}, + {[]string{"submit", "-"}, "submit", []string{"-"}, true}, + {[]string{"status", "job-123"}, "status", []string{"job-123"}, true}, + {[]string{"status", "--results", "job-123"}, "status", []string{"--results", "job-123"}, true}, + {[]string{"get", "job-123", "4"}, "get", []string{"job-123", "4"}, true}, + {[]string{"cancel", "job-123"}, "cancel", []string{"job-123"}, true}, + {[]string{}, "", nil, false}, + {[]string{"submit"}, "", nil, false}, + {[]string{"status"}, "", nil, false}, + {[]string{"status", "a", "b"}, "", nil, false}, + {[]string{"status", "--results"}, "", nil, false}, + {[]string{"get", "job-123"}, "", nil, false}, + {[]string{"get", "job-123", "x"}, "", nil, false}, + {[]string{"get", "job-123", "-1"}, "", nil, false}, + {[]string{"cancel"}, "", nil, false}, + {[]string{"bogus", "x"}, "", nil, false}, + } + for _, c := range cases { + verb, rest, err := parseBatchArgs(c.args) + if c.ok { + if err != nil { + t.Errorf("%v: unexpected error %v", c.args, err) + continue + } + if verb != c.verb || !slices.Equal(rest, c.rest) { + t.Errorf("%v: got verb=%q rest=%q", c.args, verb, rest) + } + } else if err == nil { + t.Errorf("%v: want an error", c.args) + } + } +} diff --git a/cmd/sift/batchcmd.go b/cmd/sift/batchcmd.go new file mode 100644 index 0000000..2c56d2e --- /dev/null +++ b/cmd/sift/batchcmd.go @@ -0,0 +1,200 @@ +package main + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" + "os" + "strconv" +) + +const batchUsage = `usage: sift batch submit|status|get|cancel + sift batch submit + sift batch status [--results] + sift batch get + sift batch cancel ` + +// parseBatchArgs validates a `sift batch` command line (without "batch"). verb is +// the subcommand; rest is the arguments after it, unchanged. +func parseBatchArgs(args []string) (verb string, rest []string, err error) { + if len(args) == 0 { + return "", nil, fmt.Errorf("%s", batchUsage) + } + verb, rest = args[0], args[1:] + switch verb { + case "submit": + if len(rest) != 1 { + return "", nil, fmt.Errorf("%s", batchUsage) + } + case "status": + switch len(rest) { + case 1: + if rest[0] == "--results" { + return "", nil, fmt.Errorf("%s", batchUsage) + } + case 2: + if rest[0] != "--results" { + return "", nil, fmt.Errorf("%s", batchUsage) + } + default: + return "", nil, fmt.Errorf("%s", batchUsage) + } + case "get": + if len(rest) != 2 { + return "", nil, fmt.Errorf("%s", batchUsage) + } + idx, e := strconv.Atoi(rest[1]) + if e != nil || idx < 0 { + return "", nil, fmt.Errorf("%s", batchUsage) + } + case "cancel": + if len(rest) != 1 { + return "", nil, fmt.Errorf("%s", batchUsage) + } + default: + return "", nil, fmt.Errorf("%s", batchUsage) + } + return verb, rest, nil +} + +func batchMain() { + verb, rest, err := parseBatchArgs(os.Args[1:]) + if err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(2) + } + switch verb { + case "submit": + submitMain(rest) + case "status": + statusMain(rest) + case "get": + getMain(rest) + case "cancel": + cancelMain(rest) + } +} + +// submitMain reads the targets file (or stdin when the argument is "-") and +// validates it locally before calling the server: a JSON object with a +// non-empty targets array, or a bare array of targets or URL strings wrapped +// into {"targets": [...]}. +func submitMain(rest []string) { + var r io.Reader = os.Stdin + if rest[0] != "-" { + f, err := os.Open(rest[0]) + if err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(5) + } + defer f.Close() + r = f + } + raw, err := io.ReadAll(r) + if err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(5) + } + body, err := buildTargetsBody(raw) + if err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(2) + } + batchDo(http.MethodPost, "/v1/batch", bytes.NewReader(body)) +} + +// buildTargetsBody accepts the three accepted shapes and returns the +// {"targets": [...]} body to send. A bare array of URL strings gets each string +// wrapped as {"url": s}; bare target objects pass through. +func buildTargetsBody(raw []byte) ([]byte, error) { + var probe any + if err := json.Unmarshal(raw, &probe); err != nil { + return nil, fmt.Errorf("targets: not valid JSON: %w", err) + } + switch p := probe.(type) { + case map[string]any: + arr, ok := p["targets"].([]any) + if !ok || len(arr) == 0 { + return nil, fmt.Errorf("targets: a JSON object needs a non-empty targets array") + } + return raw, nil + case []any: + if len(p) == 0 { + return nil, fmt.Errorf("targets: an empty array") + } + out := make([]any, 0, len(p)) + for _, el := range p { + switch e := el.(type) { + case map[string]any: + out = append(out, e) + case string: + out = append(out, map[string]any{"url": e}) + default: + return nil, fmt.Errorf("targets: array elements must be URLs or objects") + } + } + return json.Marshal(map[string]any{"targets": out}) + default: + return nil, fmt.Errorf("targets: expected a JSON object or array") + } +} + +func statusMain(rest []string) { + id := rest[len(rest)-1] + path := "/v1/batch/" + url.PathEscape(id) + if len(rest) == 2 && rest[0] == "--results" { + path += "?results=1" + } + batchDo(http.MethodGet, path, nil) +} + +func getMain(rest []string) { + path := "/v1/batch/" + url.PathEscape(rest[0]) + "/" + url.PathEscape(rest[1]) + batchDo(http.MethodGet, path, nil) +} + +func cancelMain(rest []string) { + batchDo(http.MethodDelete, "/v1/batch/"+url.PathEscape(rest[0]), nil) +} + +// batchDo calls base+path with the bearer header, decodes the JSON envelope, +// and prints it on stdout. On a non-2xx it writes the error to stderr and exits +// with exitFor, as fetch does. +func batchDo(method, path string, body io.Reader) { + base, key, err := loadClient() + if err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(5) + } + req, err := http.NewRequest(method, base+path, body) + if err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(5) + } + req.Header.Set("Authorization", "Bearer "+key) + if body != nil { + req.Header.Set("Content-Type", "application/json") + } + resp, err := http.DefaultClient.Do(req) + if err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(5) + } + defer resp.Body.Close() + var env map[string]any + if err := json.NewDecoder(resp.Body).Decode(&env); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(5) + } + if resp.StatusCode < 200 || resp.StatusCode > 299 { // submit answers 202 Accepted + msg, _ := env["error"].(string) + fmt.Fprintln(os.Stderr, msg) + os.Exit(exitFor(resp.StatusCode, msg)) + } + enc := json.NewEncoder(os.Stdout) + enc.SetIndent("", " ") + _ = enc.Encode(env) +} diff --git a/cmd/sift/main.go b/cmd/sift/main.go index b42c8fd..a9c83c0 100644 --- a/cmd/sift/main.go +++ b/cmd/sift/main.go @@ -26,6 +26,8 @@ func main() { extractMain() case "search": searchMain() + case "batch": + batchMain() case "keys": keysMain() case "pdftext": @@ -49,6 +51,7 @@ Commands: sift fetch fetch via the service sift extract extract a local PDF via the service sift search "" search via the service + sift batch submit|status|get|cancel run or watch a batch of fetches sift keys add|list|revoke sift version `, version) diff --git a/cmd/sift/serve.go b/cmd/sift/serve.go index 6095982..1773304 100644 --- a/cmd/sift/serve.go +++ b/cmd/sift/serve.go @@ -10,9 +10,11 @@ import ( "os/signal" "strings" "syscall" + "time" "git.wntrmute.dev/kyle/sift/internal/api" "git.wntrmute.dev/kyle/sift/internal/auth" + "git.wntrmute.dev/kyle/sift/internal/batch" "git.wntrmute.dev/kyle/sift/internal/config" "git.wntrmute.dev/kyle/sift/internal/dcache" "git.wntrmute.dev/kyle/sift/internal/digest" @@ -119,7 +121,7 @@ func serveMain() { cachePath = "/var/lib/sift/cache.db" } var cache *dcache.DB - if cfg.Dixie.Chat != "" || cfg.Search.AnyEnabled() { + if cfg.Dixie.Chat != "" || cfg.Search.AnyEnabled() || !cfg.Batch.Disabled { c, err := dcache.Open(cachePath, 500<<20, log) if err != nil { log.Warn("cache", "err", err) @@ -127,6 +129,23 @@ func serveMain() { cache = c } } + if !cfg.Batch.Disabled && cache != nil { + now := time.Now().Unix() + if n := cache.MarkInFlightFailed("restart", now); n > 0 { + for i := 0; i < n; i++ { + srv.Prom.ObserveBatchJob("restart") + } + log.Info("batch reaped", "jobs", n) + } + srv.Batch = srv.NewBatchRunner(cache, batch.Config{ + TTL: config.ParseDuration(cfg.Batch.TTL, 0), + TargetsPerJob: cfg.Batch.TargetsPerJob, + JobsPerCaller: cfg.Batch.JobsPerCaller, + JobsTotal: cfg.Batch.JobsTotal, + Workers: cfg.Batch.Workers, + MaxBytes: config.ParseSize(cfg.Batch.MaxBytes, 0), + }) + } if cfg.Dixie.Chat != "" { srv.Digest = &digest.Engine{ ChatURL: cfg.Dixie.Chat, @@ -213,8 +232,14 @@ func serveMain() { case <-sig: stop() _ = hs.Close() + if srv.Batch != nil { + srv.Batch.Close() + } case err := <-errCh: stop() + if srv.Batch != nil { + srv.Batch.Close() + } if err != nil { log.Error("serve", "err", err) os.Exit(1) diff --git a/dist/sift.toml.example b/dist/sift.toml.example index c0b426c..6ff7801 100644 --- a/dist/sift.toml.example +++ b/dist/sift.toml.example @@ -54,3 +54,12 @@ listen = "127.0.0.1:8890" # url = "https://api.tavily.com/search" # daily_credit_cap = 500 # no_rerank = true + +# [batch] +# disabled = false +# targets_per_job = 32 +# jobs_per_caller = 1 +# jobs_total = 8 +# workers = 1 # at most 2: each worker fetches one target at a time +# ttl = "24h" # a job's results live this long after its last read or write +# max_bytes = "64MiB" # stored results; the oldest finished jobs go first diff --git a/docs/implementer-log.md b/docs/implementer-log.md index 2ac42a7..498ca05 100644 --- a/docs/implementer-log.md +++ b/docs/implementer-log.md @@ -17,6 +17,15 @@ One row per task session that ends in a commit. Newest last. | p5b/03-api | 2026-09-26 | done | 1 | gate: ok | none | The digest now answers long documents itself (task 02), so the API's refusal limit is only a safety cap: `DefaultPDFMaxQuestionChunks` 200→5000 in `internal/api/pdf.go` (the 422 message text unchanged, still "document too long for question (N chunks); use grep or pages"). In `v1fetch` (`internal/api/api.go`) the digest-meta map gains `"retrieval": dr.Retrieval` and `"embedded": dr.Embedded` next to `chunks_used`/`chunks_total`, only when the digest ran without a fallback. Copied the two given tests byte-identical from the plan — the replacement `pdf_test.go` is now the protected one and asserts `DefaultPDFMaxQuestionChunks == 5000`; `pdf_retrieval_test.go` asserts the first long-doc question returns `retrieval=bm25`, `embedded=0`. README PDF section: a long-doc question is answered first from BM25 (`retrieval: "bm25"`) while its chunks embed in the background and hybrid on later questions, plus the 5000-chunk cap. One sentence added to `skill/SKILL.md`. All api tests pass under `-race -count=3`; make gate ok; given files byte-identical. | | p5c/01-extract-api | 2026-09-26 | done | 1 | gate: ok | none | `POST /v1/extract` answers a PDF posted in the body like `/v1/fetch` answers the same PDF by URL, nothing fetched. `internal/api/extract.go`: auth via `caller`; `io.ReadAll` of the body (a `*http.MaxBytesError`→413 "too large (>N MB)", empty→400); `clean.Kind` must be "pdf" else 415; query `pages`/`grep`/`question`/`mode` (digest when a question is set, else clean; other→400). Content-addressed cache: `hash="sha256:"+HashBody(body)`, `key=PDFKey("upload:"+hash, callerClass)`; a `GetPDF` hit rebuilds the doc via `pdfFromEntry` with `cached:true` + `PDFCacheHit` and no child (bypasses the TTL/revalidation that `pdfFromCache` does, so identical content is extracted once); a miss builds a synthetic `fetch.Resource{URL:"upload",...}` and runs `extractPDF`+`storePDF`. Bad pages/grep→400 via `writeFetchErr`, the chunk limit→the same 422 as `v1fetch`, extraction errors→`pdfError` via `writeFetchErr`. Envelope mirrors the PDF fetch path (`content`/`mode`/`untrusted`/`warnings`/`sizes`/`timings`/`pdf`/`digest`); `source` is `{"kind":"upload","hash","bytes","cached","fetched_at"}` with no `url` key; no Honcho context for uploads; one log line "extract" with caller/bytes/pdf_pages/mode/cached. `internal/api/api.go` scoped to the rule-1 changes: `DefaultPDFMaxUpload` const (100 MiB), `PDFMaxUpload int64` Server field, the `POST /v1/extract` route in `New`, and `ServeHTTP` (per-path upload cap instead of the 64 KiB JSON limit, and `/v1/extract` added to the `requestDeadline` switch). The `pdfMaxUpload()` field-or-default resolver lives in extract.go. Given test byte-identical; all api tests pass under `-race -count=3`; make gate ok. | | p5c/02-extract-cli | 2026-09-26 | done | 1 | gate: ok | none | `sift extract ` uploads a local PDF to `/v1/extract` and prints the answer exactly as `sift fetch` does. `cmd/sift/extractcmd.go`: `extractOpts` + `parseExtractOpts` (reuses `splitAroundPositional` with `extractValueFlags` mode/pages/grep/question, one positional file, `--json`), `extractURL` builds `base+"/v1/extract"` with `url.Values` set only for the options that are set and `mode=digest` when a question is given and no mode — values are URL-encoded so a grep like `a b&c` survives round-trip; `extractMain` reuses `loadClient`, `stat`s the file and refuses >100 MiB before uploading (exit 3, matching the server's 413→exitFor), then `http.NewRequest` with the `*os.File` as the streaming body and `ContentLength` set (no second in-memory copy), `Content-Type: application/pdf` + bearer, and the same error/exitFor path as fetch; success prints `content` (or the `--json` envelope). `main.go`: `case "extract"` and one help line. README: extract line in the CLI block plus a sentence; SKILL.md: a PDF-on-disk bullet in the PDFs section. Given test byte-identical; all cmd/sift tests pass under `-race -count=3`; make gate ok; `bin/sift extract` prints the usage line. | +| p6/01-batchstore | 2026-09-26 | done | 1 | gate: ok | none | Storage-only batch store in `internal/dcache` (new `batch.go`; schema added to `Open`; `BatchH`/`BatchM` counters added to `DB`). `CreateBatchJob` stores as given. Reads never write the lease: `BatchJob`, `BatchTargets` and the counts are pure, so exact lease values hold after a read. `BatchJobByIDem` returns the newest `(caller,idemKey)` job with `lease+ttl>now` and status queued/running/done/partial; failed/cancelled never reused. `PutBatchTargets` inserts in order; `BatchTargets` returns idx order with NULLs as nil pointers. `UpdateBatchTarget` bumps lease to `now`, increments `targets_done` exactly once per target entering done/failed (previous-status check prevents re-write double-count), and adds `len(result)` to `nbytes` when a result is written — all in one tx. `SetBatchJobStatus`/`TouchBatchJob` set lease; `SweepBatch` deletes by `lease+ttl<=now` with its targets and returns the job count; `EvictBatchOldest` evicts whole non-live jobs oldest-lease-first (never a queued/running one) until `BatchNbytes()<=maxBytes`; `MarkInFlightFailed` fails queued/running on restart; `BatchNbytes` sums job nbytes, kept out of `Size()`. Accessors return `(v,bool)`/`error`, count misses in `BatchM`, reuse `ClassFleet`/`ClassRestricted`. A first pass failed `TestBatchIDemLookup` because I forgot `.Scan(&id)` on the `QueryRow` (the un-scanned `*sql.Row` was always non-nil, so the method always missed); fixed and verified. All dcache tests pass under `-race -count=3`; make gate ok; given file byte-identical. +| p6/02-runner-core | 2026-09-26 | done | 1 | gate: ok | caller-cleanup hand-off to task 03 | Runner core in new internal/batch with no workers (task 03 adds them; jobs stay queued, so a queued job counts as live for the caps). Public surface in batch.go: Options/Target/Result/TargetError, FetchFunc, Job/TargetStatus, Hooks and Config with defaults (TTL 24h, 32 targets/job, 1 job/caller, 8 total, worker clamped to 2, 64 MiB, 2 s/10 m retry). Submit runs the given order — empty→ErrNoTargets, idempotency via hex SHA-256 of `name+NUL+json.Marshal(targets)` (a live hit returns the id reused, touching no caps), then TargetsPerJob/JobsPerCaller/JobsTotal caps, then stores the job queued (random 16-byte hex id, CreatedAt=Lease=now) and its targets (pending, Options=json of t.Options) before returning so a queued job is live; the submitting caller is held in memory by job id, never persisted. Status maps rows and decodes Result 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/host. | +| p6/03-runner-workers | 2026-09-26 | done | 1 | gate: ok | none | Workers in new internal/batch/worker.go: one rotation of live jobs in submit order, round-robin via an absolute cursor (advances in pick, wraps at access, so it survives a second job being added while the first is in flight — the mod-n cursor from the design stuck at 0 for one job and re-picked it), serial per job (target i+1 never starts before i is stored), one target per fetch under the Close-cancelled context (panic→ErrInternal, never echoed; ErrRetryLater retried after RetryWait, bounded by RetryFor then stored failed with the wrapped text). Terminal status once per job: all done→done, all failed→failed, mix→partial. Cancel of a running job sets cancelled, stores the in-flight target without overwriting cancelled, leaves later targets pending. Close sets closed, cancels the context, broadcasts, waits the workers, idempotent. batch.go: New starts cfg.Workers, Submit appends the liveJob and wakes, Cancel handles the in-flight case, Close stops. Absolute cursor is the one deviation from the design's "index in live" comment. All race tests pass -count=3. | | +| p6/04-metrics | 2026-09-26 | done | 1 | gate: ok | none | Batch series in new internal/metrics/batch.go, registered in New: `BatchJobsTotal`/`BatchTargetsTotal` CounterVecs (label "outcome", any value outside the known set folded to "other" via `batchJobOutcomes`/`batchTargetOutcomes` maps so a raw string never becomes a label), `BatchQueueDepth`/`BatchTargetsInflight` gauges, and `BatchJobSeconds`/`BatchTargetSeconds` histograms (durationBuckets) plus `BatchResultBytes` (1K,4K,16K,64K,256K,1M). Nil-receiver-safe `Observe…`/`Set…` methods mirroring ObservePDF/ObserveDigest. Series served through the existing /metrics handler; no new endpoint, no Snapshot change. Given test byte-identical; metrics tests pass under -race -count=3; make gate ok. | +| p6/05-fetchtarget | 2026-09-26 | done | 1 | gate: ok | none | Extracted v1fetch's per-target pipeline into new `internal/api/target.go`: `fetchTarget(ctx, c, t batch.Target) (batch.Result, error)`. Body moved unchanged in order — question truncation to 500 runes, mode default/check, wantCtx, doFetch, recover path, PDF branch (render, pages/grep errors, question-chunk limit), clean/raw branch, digest, context search, the fetch log line, and the envelope. `content` is returned as `Result.Content`; every other envelope key (`mode`,`untrusted`,`source`,`warnings`,`sizes`,`timings`, and `pdf`/`digest`/`context` when present) goes in `Result.Meta`, never content. `v1fetch` keeps reqN/caller/decode/400, builds a `batch.Target` (`URL` + `Mode`/`Question`/`Pages`/`Grep`/`JQ`/`Context`/`Recover` into `Options`), calls `fetchTarget`, `writeFetchErr` on error and `writeJSON(w,200,env)` on success where `env` is a fresh copy of `Meta` plus `"content"` (a copy, so `len(env)==len(Meta)+1` holds and Meta is not mutated). The two `writeErr` failures become `*fetch.Error` with the same status/msg/hint: 400 "invalid url" (bad mode), 400 "raw mode does not apply to PDFs". `doFetch` takes `context.Context` instead of `*http.Request` (its three `r.Context()` → `ctx`); `scrape` passes `r.Context()`. Removed now-unused `fmt`/`unicode/utf8`/`recover` imports, added `batch`. All api tests pass under -race -count=3; make gate ok; given test byte-identical. | +| p6/06-batch-wiring | 2026-09-26 | done | 1 | gate: ok | none | New `internal/api/batch.go`: `NewBatchRunner` overwrites `cfg.Hooks` so Job→ObserveBatchJob, Target→ObserveBatchTarget(batchOutcome)+ObserveBatchTargetSeconds (+ObserveBatchResultBytes only when err==nil), and JobSeconds/Depth/Inflight→the matching `s.Prom` methods; the runner's FetchFunc is `batchFetch`, which tags ctx with an unexported `batchCtxKey`, runs under `requestDeadline`, and maps fetchTarget's errors (a *fetch.Error 429/503 → `fmt.Errorf("%s: %w", fe.Msg, batch.ErrRetryLater)` so the runner retries; any other *fetch.Error → TargetError with 502 rewritten to 500 "internal error" and `s.blockN.Add(1)` for 403; anything else → TargetError 500 logged Error with `s.logErr`). `batchOutcome` classifies nil→ok, ErrRetryLater-wrapped→busy, then by TargetError.Status (403 blocked, 429 busy, 504 timeout, 415 unsupported, 422 whose Msg starts "document too long"→too_long, other 4xx→upstream_4xx, 5xx→upstream_5xx, any other error→upstream_5xx). Added `Batch *batch.Runner` Server field (nil → the /v1/batch endpoints answer 503 "batch disabled"). `pdf.go` extractPDF charges the PDF pool to `c.Name+"/batch"` when the ctx carries the batch mark, single-flight key unchanged so a batch and an interactive extract of the same PDF still share one child; the only new log line carries caller and host="-" (never a URL/host). All api tests pass under -race -count=3; make gate ok; given file byte-identical. | +| p6/07-api-endpoints | 2026-09-26 | done | 1 | gate: ok | none | The four /v1/batch handlers in new `internal/api/batch.go`: `batchSubmit` (POST) decodes `{"targets":[{url,mode,question,pages,grep,jq,context,recover}]}` flat like `/v1/fetch`, 400 "invalid batch" on bad json/empty list/empty url/bad mode (only empty,clean,raw,digest), then `PreCheck` per target → 422 "blocked: target : " with no job, then `Submit` mapping ErrTooManyTargets→422 "too many targets (max 32)", ErrCallerBusy→429+Retry-After 30, ErrBusy→429+Retry-After 60, ErrNoTargets→400, and 202 {success,job_id,status,targets,reused} (status "queued" new, current status reused). `batchStatus` (GET) and `batchResult` (GET {id}/{idx}) and `batchCancel` (DELETE) share `ownership` (unknown or not `c.Name` → 404, never 403; nil batch → 503); status is a summary with no results unless `?results=1` inlines the envelope (Meta+content) on done targets, idx validated 0..targets_total-1 else 404, cancel of a terminal job → 409 "job already finished" else 200 {success,status:cancelled}. `api.go` scoped to the rule-2 changes: `batchMaxBody` (256 KiB) const, the four routes in `New`, and `ServeHTTP` applying the per-path cap once (`/v1/batch`→256 KiB, `/v1/extract`→pdfMaxUpload, else 64 KiB) and collapsing every batch path to a fixed metrics label via `batchObservePath` (`/v1/batch` or `/v1/batch/{id}`, never a job id). Each handler logs one `batch` line with caller/job_id/targets/status-or-reused and never a URL/hostname/idem_key. Given test byte-identical; all api tests pass under -race -count=3; make gate ok. | +| p6/08-serve-config | 2026-09-26 | done | 1 | gate: ok | none | `[batch]` config in `internal/config`: the `Batch` type (disabled/targets_per_job/jobs_per_caller/jobs_total/workers/ttl/max_bytes) and `File.Batch`, plus `ParseSize` (trims spaces, accepts only binary KiB/MiB/GiB suffixes or a plain byte count, zero/negative/other → def). serve.go opens the dcache store when batch is on (`!cfg.Batch.Disabled`) alongside dixie/search — same path, same call, Warn on failure — then, before building the runner, reaps in-flight jobs a restart killed via `MarkInFlightFailed("restart", now)` with one `ObserveBatchJob("restart")` per reaped job and a `batch reaped` info log when n>0; builds `srv.Batch` from the config (TTL via `ParseDuration`, MaxBytes via `ParseSize`, runner defaults fill the zeros) only when enabled and the store opened; closes `srv.Batch` in both shutdown branches when non-nil. Added a commented `[batch]` block to dist/sift.toml.example. Given test byte-identical; all config and cmd tests pass under -race -count=3. | +| p6/09-cli-docs | 2026-09-26 | done | 1 | gate: ok | none | `cmd/sift/batchcmd.go`: `parseBatchArgs` (verb + unchanged `rest`; submit takes one arg, status one job id optionally preceded by `--results`, get a job id plus a non-negative-integer idx, cancel one job id, else a usage error), `batchMain` dispatch, one small function per verb, and `batchDo` (bearer header, decode the JSON envelope, print it indented on stdout, on non-2xx print `env["error"]` to stderr and `exitFor`). `submitMain` reads the file or stdin and `buildTargetsBody` accepts a JSON object with a non-empty `targets` array, a bare array of target objects, or a bare array of URL strings (each wrapped `{"url": s}`), validating locally before the call. `main.go`: one `case "batch"` and one help line. README "Batch" section (four endpoints, serial-per-job/one-worker/round-robin model, 32 targets/1-per-caller/8-total caps, 256 KiB body, idempotency, lease/TTL/restart, summary-vs-results, restricted redaction, `sift_batch_*` metrics); SKILL.md "Batch" section. Given test byte-identical; all cmd/sift tests pass under -race -count=3. | ## P5a review (owner, 2026-09-26) diff --git a/internal/api/api.go b/internal/api/api.go index 9c7d4b0..61a0a2b 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -6,7 +6,6 @@ import ( "encoding/hex" "encoding/json" "errors" - "fmt" "io" "log/slog" "net/http" @@ -16,9 +15,9 @@ import ( "sync" "sync/atomic" "time" - "unicode/utf8" "git.wntrmute.dev/kyle/sift/internal/auth" + "git.wntrmute.dev/kyle/sift/internal/batch" "git.wntrmute.dev/kyle/sift/internal/clean" "git.wntrmute.dev/kyle/sift/internal/dcache" "git.wntrmute.dev/kyle/sift/internal/digest" @@ -29,7 +28,6 @@ import ( "git.wntrmute.dev/kyle/sift/internal/pdfdoc" "git.wntrmute.dev/kyle/sift/internal/pdfpool" "git.wntrmute.dev/kyle/sift/internal/pdftext" - "git.wntrmute.dev/kyle/sift/internal/recover" "git.wntrmute.dev/kyle/sift/internal/search" "git.wntrmute.dev/kyle/sift/internal/selfip" ) @@ -39,6 +37,10 @@ const ( writeTimeout = 240 * time.Second // P5a: a PDF may take 120 s to download + 20 s queued + 60 s to extract requestDeadline = 210 * time.Second // P5a: the PDF path's worst case (see writeTimeout); other paths keep their own shorter timeouts + // batchMaxBody is the largest body POST /v1/batch accepts: 32 targets, each with a + // large question, exceed the 64 KiB cap of the other endpoints. + batchMaxBody = 256 << 10 + // DefaultPDFMaxUpload is the largest body POST /v1/extract accepts: an uploaded // PDF, addressed by its hash and extracted once per content. DefaultPDFMaxUpload = 100 << 20 @@ -75,6 +77,10 @@ type Server struct { pdfPoolOnce sync.Once + // Batch is the batch runner; nil means the /v1/batch endpoints (task 07) answer 503 + // "batch disabled". + Batch *batch.Runner + // uploads bounds how many /v1/extract bodies (up to PDFMaxUpload each) are read into // memory at once, ahead of the PDF pool's own limits. uploads chan struct{} @@ -106,6 +112,10 @@ func New(version string, g *guard.Guard, f *fetch.Fetcher, keys *auth.Store, log s.Mux.HandleFunc("POST /v1/extract", s.v1extract) s.Mux.HandleFunc("POST /v1/search", s.v1search) s.Mux.HandleFunc("POST /v2/search", s.v2search) + s.Mux.HandleFunc("POST /v1/batch", s.batchSubmit) + s.Mux.HandleFunc("GET /v1/batch/{id}", s.batchStatus) + s.Mux.HandleFunc("GET /v1/batch/{id}/{idx}", s.batchResult) + s.Mux.HandleFunc("DELETE /v1/batch/{id}", s.batchCancel) return s } @@ -123,8 +133,11 @@ func (s *Server) HTTPServer(addr string) *http.Server { func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { var limit int64 = maxReqBody - if r.URL.Path == "/v1/extract" { + switch r.URL.Path { + case "/v1/extract": limit = int64(s.pdfMaxUpload()) + case "/v1/batch": + limit = batchMaxBody } r.Body = http.MaxBytesReader(w, r.Body, limit) switch r.URL.Path { @@ -142,9 +155,29 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { s.Prom.Observe(r.URL.Path, time.Since(start), code) return } + if strings.HasPrefix(r.URL.Path, "/v1/batch") { + cw := &codeWriter{ResponseWriter: w} + start := time.Now() + s.Mux.ServeHTTP(cw, r) + code := cw.code + if code == 0 { + code = http.StatusOK + } + s.Prom.Observe(batchObservePath(r.URL.Path), time.Since(start), code) + return + } s.Mux.ServeHTTP(w, r) } +// batchObservePath collapses a batch path to a fixed label so a job id never becomes a +// metric label value: the submit path stays, every {id}/{idx} variant becomes {id}. +func batchObservePath(path string) string { + if path == "/v1/batch" { + return "/v1/batch" + } + return "/v1/batch/{id}" +} + type codeWriter struct { http.ResponseWriter code int @@ -389,7 +422,7 @@ func (s *Server) scrape(w http.ResponseWriter, r *http.Request) { } } s.scrapeN.Add(1) - res, cl, doc, ferr := s.doFetch(r, c, req.URL, "", onlyMain) + res, cl, doc, ferr := s.doFetch(r.Context(), c, req.URL, "", onlyMain) if ferr != nil { s.writeFetchErr(w, c, req.URL, ferr) return @@ -449,222 +482,29 @@ func (s *Server) v1fetch(w http.ResponseWriter, r *http.Request) { writeErr(w, 400, "invalid url", "") return } - q := req.Question - if len([]rune(q)) > 500 { - rns := []rune(q) - q = string(rns[:500]) - } - mode := req.Mode - if mode == "" { - if q != "" { - mode = "digest" - } else { - mode = "clean" - } - } - if mode != "clean" && mode != "raw" && mode != "digest" { - writeErr(w, 400, "invalid url", "") - return - } - wantCtx := q != "" - if req.Context != nil { - wantCtx = *req.Context - } - - t0 := time.Now() - onlyMain := true - res, cl, doc, ferr := s.doFetch(r, c, req.URL, req.JQ, onlyMain) - var snap *recover.Result - if ferr != nil && req.Recover && recoverWorthy(ferr) { - sr, rerr := recover.Try(r.Context(), s.Fetch, req.URL) - if rerr == nil && sr != nil && sr.Res != nil { - res, cl, doc, ferr = s.cleanFetched(sr.Res, req.JQ, onlyMain) - snap = sr - } - } - if ferr != nil { - s.writeFetchErr(w, c, req.URL, ferr) - return - } - fetchMS := time.Since(t0).Milliseconds() - - if doc == nil && (req.Pages != "" || req.Grep != "") { - s.writeFetchErr(w, c, req.URL, &fetch.Error{Status: 400, Msg: "pages and grep apply to PDFs only"}) - return - } - - var docLen, pdfPages int - var content string - var pdfMap map[string]any - if doc != nil { - pdfPages = doc.Pages - if mode == "raw" { - writeErr(w, 400, "raw mode does not apply to PDFs", "") - return - } - rp, err := s.renderPDF(doc, pdfdoc.Options{Pages: req.Pages, Grep: req.Grep, OutCap: s.OutCap}) - if err != nil { - if errors.Is(err, pdfdoc.ErrBadPages) || errors.Is(err, pdfdoc.ErrBadGrep) { - s.writeFetchErr(w, c, req.URL, &fetch.Error{Status: 400, Msg: err.Error()}) - return - } - s.writeFetchErr(w, c, req.URL, err) - return - } - full := pdfdoc.Full(doc) - docLen = utf8.RuneCountInString(full) - if mode == "digest" { - content = full - } else { - content = rp.Content - } - pdfMap = map[string]any{ - "pages": doc.Pages, - "title": doc.Title, - "outline": doc.Outline, - "extracted": len(doc.Text), - "first": rp.First, - "last": rp.Last, - "map": rp.Map, - "grep_hits": rp.GrepHits, - "note": rp.Note, - } - } else { - content = cl.Markdown - if cl.Kind == "json" { - content = cl.HTML - } - if mode == "raw" { - content = string(res.Body) - if len(content) > s.OutCap { - content = clean.TruncateUTF8(content, s.OutCap) - cl.Trunc = true - cl.Warning = "truncated to output cap" - } - } - } - warn := []string{} - if cl.Warning != "" { - warn = append(warn, cl.Warning) - } - - var digMeta map[string]any - var digestMS int64 - if mode == "digest" { - if doc != nil { - n := len(digest.Split(pdfdoc.Full(doc), digest.ChunkTokens)) - limit := s.PDFMaxQuestionChunks - if limit == 0 { - limit = DefaultPDFMaxQuestionChunks - } - if n > limit { - s.writeFetchErr(w, c, req.URL, &fetch.Error{Status: 422, Msg: fmt.Sprintf("document too long for question (%d chunks); use grep or pages", n)}) - return - } - } - if s.Digest == nil { - warn = append(warn, "digest fell back to clean: unset") - mode = "clean" - content, warn = capDigestFallback(content, warn) - digMeta = map[string]any{"fallback": "unset"} - } else { - dr := s.Digest.Run(r.Context(), content, q, res.FinalURL, s.callerClass(c)) - digestMS = dr.DigestMS - if !dr.Cached { - s.Prom.ObserveDigest(time.Duration(dr.DigestMS) * time.Millisecond) - } - if dr.Fallback != "" { - warn = append(warn, "digest fell back to clean: "+dr.Fallback) - mode = "clean" - content, warn = capDigestFallback(content, warn) - digMeta = map[string]any{"fallback": dr.Fallback, "model": dr.Model} - } else { - content = dr.Text - digMeta = map[string]any{ - "model": dr.Model, - "chunks_used": dr.ChunksUsed, - "chunks_total": dr.ChunksTotal, - "retrieval": dr.Retrieval, - "embedded": dr.Embedded, - "fallback": false, - "cached": dr.Cached, - } - } - } - } - - var ctxItems []fleetctx.Item - var ctxMS int64 - if wantCtx && s.Ctx != nil { - ct0 := time.Now() - query := fleetctx.Query(q, cl.Title, headings(cl.Markdown)) - items, cw := s.Ctx.Search(r.Context(), c.Workspace, c.Name, query) - ctxMS = time.Since(ct0).Milliseconds() - if cw != "" { - warn = append(warn, cw) - } - ctxItems = items - } - - qLen := len(q) - if qLen > 80 { - qLen = 80 - } - fb, _ := "", false - if digMeta != nil { - if s, ok := digMeta["fallback"].(string); ok { - fb = s - } - } - s.Log.Info("fetch", "caller", c.Name, "host", s.logHost(c, res.FinalURL), "mode", mode, - "q_len", qLen, "raw_tokens", len(res.Body)/4, "returned_tokens", len(content)/4, - "pdf_pages", pdfPages, "digest_ms", digestMS, "fallback", fb) - - src := map[string]any{ - "url": res.URL, - "final_url": res.FinalURL, - "status": res.Status, - "content_type": res.ContentType, - "fetched_at": res.FetchedAt.Format(time.RFC3339), - "hash": res.SHA256(), - "bytes": len(res.Body), - "cached": len(res.Body) == 0, - "kind": "live", - } - if snap != nil { - src["kind"] = "snapshot" - src["via"] = string(snap.Via) - src["original_url"] = snap.OriginalURL - if snap.SnapshotAt != "" { - src["snapshot_at"] = snap.SnapshotAt - } else { - src["snapshot_at"] = nil - warn = append(warn, "snapshot date unknown") - } - } - out := map[string]any{ - "content": content, - "mode": mode, - "untrusted": true, - "source": src, - "warnings": warn, - "sizes": map[string]int{ - "raw_tokens": docLen / 4, - "clean_tokens": digest.EstTokens(cl.Markdown), - "returned_tokens": digest.EstTokens(content), + t := batch.Target{ + URL: req.URL, + Options: batch.Options{ + Mode: req.Mode, + Question: req.Question, + Pages: req.Pages, + Grep: req.Grep, + JQ: req.JQ, + Context: req.Context, + Recover: req.Recover, }, - "timings": map[string]int64{"fetch_ms": fetchMS, "digest_ms": digestMS, "context_ms": ctxMS}, } - if pdfMap != nil { - out["pdf"] = pdfMap + res, err := s.fetchTarget(r.Context(), c, t) + if err != nil { + s.writeFetchErr(w, c, req.URL, err) + return } - if digMeta != nil { - out["digest"] = digMeta + env := make(map[string]any, len(res.Meta)+1) + for k, v := range res.Meta { + env[k] = v } - if wantCtx { - out["context"] = ctxItems - } - writeJSON(w, 200, out) + env["content"] = res.Content + writeJSON(w, 200, env) } func recoverWorthy(err error) bool { @@ -710,8 +550,8 @@ func (s *Server) cleanFetched(res *fetch.Resource, jq string, onlyMain bool) (*f return res, cl, nil, nil } -func (s *Server) doFetch(r *http.Request, c *auth.Caller, rawURL, jq string, onlyMain bool) (*fetch.Resource, *clean.Result, *pdftext.Doc, error) { - res, hitDoc, ok, err := s.pdfFromCache(r.Context(), c, rawURL) +func (s *Server) doFetch(ctx context.Context, c *auth.Caller, rawURL, jq string, onlyMain bool) (*fetch.Resource, *clean.Result, *pdftext.Doc, error) { + res, hitDoc, ok, err := s.pdfFromCache(ctx, c, rawURL) if err != nil { return nil, nil, nil, err } @@ -724,7 +564,7 @@ func (s *Server) doFetch(r *http.Request, c *auth.Caller, rawURL, jq string, onl } defer s.release(c.Name, rawURL) - res, err = s.Fetch.Fetch(r.Context(), rawURL) + res, err = s.Fetch.Fetch(ctx, rawURL) if err != nil { return nil, nil, nil, err } @@ -756,7 +596,7 @@ func (s *Server) doFetch(r *http.Request, c *auth.Caller, rawURL, jq string, onl } var doc *pdftext.Doc if kind == "pdf" { - doc, err = s.extractPDF(r.Context(), c, res) + doc, err = s.extractPDF(ctx, c, res) if err != nil { return nil, nil, nil, err } diff --git a/internal/api/batch.go b/internal/api/batch.go new file mode 100644 index 0000000..50f7aac --- /dev/null +++ b/internal/api/batch.go @@ -0,0 +1,372 @@ +package api + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/http" + "strconv" + "strings" + "time" + + "git.wntrmute.dev/kyle/sift/internal/auth" + "git.wntrmute.dev/kyle/sift/internal/batch" + "git.wntrmute.dev/kyle/sift/internal/dcache" + "git.wntrmute.dev/kyle/sift/internal/fetch" +) + +// batchCtxKey marks a context as a batch fetch so extractPDF charges the PDF pool to the +// caller's "/batch" slot instead of its interactive one. +type batchCtxKey struct{} + +func isBatchFetch(ctx context.Context) bool { + ok, _ := ctx.Value(batchCtxKey{}).(bool) + return ok +} + +// NewBatchRunner builds the runner that serve (task 07) and the tests use. Its FetchFunc is +// s.batchFetch and its Hooks feed s.Prom; any cfg.Hooks from the caller is overwritten. +func (s *Server) NewBatchRunner(db *dcache.DB, cfg batch.Config) *batch.Runner { + cfg.Hooks = batch.Hooks{ + Job: func(outcome string) { + s.Prom.ObserveBatchJob(outcome) + }, + Target: func(err error, d time.Duration, n int) { + s.Prom.ObserveBatchTarget(batchOutcome(err)) + s.Prom.ObserveBatchTargetSeconds(d) + if err == nil { + s.Prom.ObserveBatchResultBytes(n) + } + }, + JobSeconds: func(d time.Duration) { + s.Prom.ObserveBatchJobSeconds(d) + }, + Depth: func(live int) { + s.Prom.SetBatchQueueDepth(live) + }, + Inflight: func(n int) { + s.Prom.SetBatchTargetsInflight(n) + }, + } + return batch.New(db, s.batchFetch, cfg, s.Log) +} + +// batchFetch is the runner's FetchFunc: one target through s.fetchTarget under the request +// deadline, tagged as a batch fetch. Its errors map to what the runner and the metrics need. +func (s *Server) batchFetch(ctx context.Context, c *auth.Caller, t batch.Target) (batch.Result, error) { + ctx, cancel := context.WithTimeout(ctx, requestDeadline) + defer cancel() + ctx = context.WithValue(ctx, batchCtxKey{}, true) + res, err := s.fetchTarget(ctx, c, t) + if err != nil { + var fe *fetch.Error + if errors.As(err, &fe) { + switch fe.Status { + case 429, 503: + return batch.Result{}, fmt.Errorf("%s: %w", fe.Msg, batch.ErrRetryLater) + } + // Copy, never mutate: the PDF pool's single-flight may share this error with an + // interactive request. + status, msg := fe.Status, fe.Msg + if status == 502 { + status, msg = 500, "internal error" + } + if status == 403 { + s.blockN.Add(1) + } + return batch.Result{}, &batch.TargetError{Status: status, Msg: msg, Hint: fe.Hint} + } + s.Log.Error("internal", "caller", c.Name, "host", "-", "err", s.logErr(c, err)) + return batch.Result{}, &batch.TargetError{Status: 500, Msg: "internal error"} + } + return res, nil +} + +// batchOutcome classifies a finished target's error into the sift_batch_targets_total label: +// a nil error is ok, a retry-later the runner gave up on is busy, a TargetError by its status, +// and anything else upstream_5xx so a raw string never becomes a label. +func batchOutcome(err error) string { + switch { + case err == nil: + return "ok" + case errors.Is(err, batch.ErrRetryLater): + return "busy" + } + var te *batch.TargetError + if errors.As(err, &te) { + switch te.Status { + case 403: + return "blocked" + case 429: + return "busy" + case 504: + return "timeout" + case 415: + return "unsupported" + case 422: + if strings.HasPrefix(te.Msg, "document too long") { + return "too_long" + } + return "upstream_4xx" + } + if te.Status >= 400 && te.Status < 500 { + return "upstream_4xx" + } + } + return "upstream_5xx" +} + +// The four /v1/batch handlers (task 07). A job that is unknown or owned by another caller is a +// 404 for every GET and DELETE, never a 403, so a caller cannot probe for another caller's jobs. + +// ownership answers the caller for id: a disabled batch is 503, an unknown id or one not owned by +// c is a 404. The empty job returned on either path signals "already answered" to the caller. +func (s *Server) ownership(w http.ResponseWriter, c *auth.Caller, id string) (batch.Job, []batch.TargetStatus) { + if s.Batch == nil { + writeErr(w, 503, "batch disabled", "") + return batch.Job{}, nil + } + j, ts, ok := s.Batch.Status(id) + if !ok || j.Caller != c.Name { + writeErr(w, 404, "not found", "") + return batch.Job{}, nil + } + return j, ts +} + +// logBatch writes the one batch line per handler: caller, job_id, targets, then either status or +// reused. It never carries a target URL, hostname or the idem_key. +func (s *Server) logBatch(c *auth.Caller, jobID string, targets int, extra ...any) { + s.Log.Info("batch", + append([]any{ + "caller", c.Name, + "job_id", jobID, + "targets", targets, + }, extra...)..., + ) +} + +func validBatchMode(m string) bool { + switch m { + case "", "clean", "raw", "digest": + return true + } + return false +} + +func (s *Server) batchSubmit(w http.ResponseWriter, r *http.Request) { + c := s.caller(w, r) + if c == nil { + return + } + if s.Batch == nil { // [batch] disabled, or the store did not open + writeErr(w, 503, "batch disabled", "") + return + } + var req struct { + Targets []v1Req `json:"targets"` + } + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + writeErr(w, 400, "invalid batch: bad json", "") + return + } + if len(req.Targets) == 0 { + writeErr(w, 400, "invalid batch: no targets", "") + return + } + targets := make([]batch.Target, 0, len(req.Targets)) + for _, t := range req.Targets { + if t.URL == "" { + writeErr(w, 400, "invalid batch: empty url", "") + return + } + if !validBatchMode(t.Mode) { + writeErr(w, 400, "invalid batch: bad mode", "") + return + } + targets = append(targets, batch.Target{ + URL: t.URL, + Options: batch.Options{ + Mode: t.Mode, + Question: t.Question, + Pages: t.Pages, + Grep: t.Grep, + JQ: t.JQ, + Context: t.Context, + Recover: t.Recover, + }, + }) + } + for i, t := range targets { + if err := s.Guard.PreCheck(t.URL); err != nil { + s.logBatch(c, "", len(targets), "status", "blocked") + writeErr(w, 422, fmt.Sprintf("blocked: target %d: %v", i, err), "") + return + } + } + id, reused, err := s.Batch.Submit(r.Context(), c, s.callerClass(c), targets) + if err != nil { + s.logBatch(c, "", len(targets), "status", "failed") + switch { + case errors.Is(err, batch.ErrTooManyTargets): + writeErr(w, 422, "too many targets (max 32)", "") + case errors.Is(err, batch.ErrCallerBusy): + w.Header().Set("Retry-After", "30") + writeErr(w, 429, "busy: a batch of yours is still running", "") + case errors.Is(err, batch.ErrBusy): + w.Header().Set("Retry-After", "60") + writeErr(w, 429, "batch jobs busy", "") + case errors.Is(err, batch.ErrNoTargets): + writeErr(w, 400, "invalid batch: no targets", "") + default: + writeErr(w, 500, "internal error", "") + } + return + } + status := "queued" + if reused { + if j, _, ok := s.Batch.Status(id); ok { + status = j.Status + } + } + if reused { + s.logBatch(c, id, len(targets), "reused", true) + } else { + s.logBatch(c, id, len(targets), "status", status) + } + writeJSON(w, 202, map[string]any{ + "success": true, + "job_id": id, + "status": status, + "targets": len(targets), + "reused": reused, + }) +} + +func (s *Server) batchStatus(w http.ResponseWriter, r *http.Request) { + c := s.caller(w, r) + if c == nil { + return + } + id := r.PathValue("id") + j, ts := s.ownership(w, c, id) + if j.ID == "" { + return + } + s.Batch.Touch(id) + s.logBatch(c, id, j.TargetsTotal, "status", j.Status) + writeJSON(w, 200, statusEnvelope(j, ts, r)) +} + +func (s *Server) batchResult(w http.ResponseWriter, r *http.Request) { + c := s.caller(w, r) + if c == nil { + return + } + id := r.PathValue("id") + j, ts := s.ownership(w, c, id) + if j.ID == "" { + return + } + idx, err := strconv.Atoi(r.PathValue("idx")) + if err != nil || idx < 0 || idx >= j.TargetsTotal || idx >= len(ts) { + writeErr(w, 404, "not found", "") + return + } + s.Batch.Touch(id) + t := ts[idx] + out := map[string]any{ + "job_id": j.ID, + "idx": t.Idx, + "url": t.URL, + "status": t.Status, + } + if t.Result != nil { + out["result"] = resultEnvelope(t.Result) + } + if t.Error != "" { + out["error"] = t.Error + } + if t.Hint != "" { + out["hint"] = t.Hint + } + s.logBatch(c, id, j.TargetsTotal, "status", t.Status) + writeJSON(w, 200, out) +} + +func (s *Server) batchCancel(w http.ResponseWriter, r *http.Request) { + c := s.caller(w, r) + if c == nil { + return + } + id := r.PathValue("id") + j, _ := s.ownership(w, c, id) + if j.ID == "" { + return + } + switch j.Status { + case "done", "partial", "failed", "cancelled": + s.logBatch(c, id, j.TargetsTotal, "status", "failed") + writeErr(w, 409, "job already finished", "") + return + } + if s.Batch.Cancel(id) { + s.logBatch(c, id, j.TargetsTotal, "status", "cancelled") + writeJSON(w, 200, map[string]any{"success": true, "status": "cancelled"}) + return + } + s.logBatch(c, id, j.TargetsTotal, "status", "failed") + writeErr(w, 409, "job already finished", "") +} + +// statusEnvelope is the summary GET /v1/batch/{id} returns: statuses, never inline results, +// because 32 results can be several megabytes. With ?results=1 each done target carries the +// /v1/fetch envelope (Meta plus content). +func statusEnvelope(j batch.Job, ts []batch.TargetStatus, r *http.Request) map[string]any { + out := map[string]any{ + "job_id": j.ID, + "status": j.Status, + "targets_total": j.TargetsTotal, + "targets_done": j.TargetsDone, + "created_at": time.Unix(j.CreatedAt, 0).UTC().Format(time.RFC3339), + "targets": targetSummaries(ts, r), + } + if j.Error != "" { + out["error"] = j.Error + } + return out +} + +func targetSummaries(ts []batch.TargetStatus, r *http.Request) []map[string]any { + results := r.URL.Query().Get("results") == "1" + out := make([]map[string]any, 0, len(ts)) + for _, t := range ts { + m := map[string]any{ + "idx": t.Idx, + "url": t.URL, + "status": t.Status, + } + if t.Error != "" { + m["error"] = t.Error + } + if t.Hint != "" { + m["hint"] = t.Hint + } + if results && t.Status == "done" && t.Result != nil { + m["result"] = resultEnvelope(t.Result) + } + out = append(out, m) + } + return out +} + +// resultEnvelope is the /v1/fetch shape: every Meta field plus content. +func resultEnvelope(res *batch.Result) map[string]any { + env := make(map[string]any, len(res.Meta)+1) + for k, v := range res.Meta { + env[k] = v + } + env["content"] = res.Content + return env +} diff --git a/internal/api/batch_review_test.go b/internal/api/batch_review_test.go new file mode 100644 index 0000000..0e526be --- /dev/null +++ b/internal/api/batch_review_test.go @@ -0,0 +1,29 @@ +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()) + } + } +} diff --git a/internal/api/batch_test.go b/internal/api/batch_test.go new file mode 100644 index 0000000..6b81ad6 --- /dev/null +++ b/internal/api/batch_test.go @@ -0,0 +1,280 @@ +package api + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" +) + +// Every test fetches only from local httptest servers or internal.test (fake DNS): no test +// touches the network. newBatchServer, pageServer and fakeDNS come from batchrun_test.go. + +func batchReq(t *testing.T, s *Server, method, path, key string, body any) *httptest.ResponseRecorder { + t.Helper() + var rd io.Reader + if body != nil { + b, err := json.Marshal(body) + if err != nil { + t.Fatal(err) + } + rd = strings.NewReader(string(b)) + } + req := httptest.NewRequest(method, path, rd) + req.Header.Set("Authorization", "Bearer "+key) + if body != nil { + req.Header.Set("Content-Type", "application/json") + } + rec := httptest.NewRecorder() + s.ServeHTTP(rec, req) + return rec +} + +func batchDecode(t *testing.T, rec *httptest.ResponseRecorder) map[string]any { + t.Helper() + var m map[string]any + if err := json.Unmarshal(rec.Body.Bytes(), &m); err != nil { + t.Fatalf("decode: %v (%d %s)", err, rec.Code, rec.Body.String()) + } + return m +} + +func submit(t *testing.T, s *Server, key string, targets ...map[string]any) map[string]any { + t.Helper() + rec := batchReq(t, s, http.MethodPost, "/v1/batch", key, map[string]any{"targets": targets}) + if rec.Code != 202 { + t.Fatalf("submit: %d %s", rec.Code, rec.Body.String()) + } + return batchDecode(t, rec) +} + +func waitTerminal(t *testing.T, s *Server, key, id string) map[string]any { + t.Helper() + deadline := time.Now().Add(10 * time.Second) + for time.Now().Before(deadline) { + env := batchDecode(t, batchReq(t, s, http.MethodGet, "/v1/batch/"+id, key, nil)) + switch env["status"] { + case "done", "partial", "failed", "cancelled": + return env + } + time.Sleep(5 * time.Millisecond) + } + t.Fatal("batch never reached a terminal status") + return nil +} + +func TestBatchSubmitAndCollect(t *testing.T) { + ts := pageServer(t, nil) + s, keys := newBatchServer(t, "test") + sub := submit(t, s, keys["test"], map[string]any{"url": ts.URL + "/one"}, map[string]any{"url": ts.URL + "/two"}) + id, _ := sub["job_id"].(string) + if sub["success"] != true || id == "" || sub["reused"] != false || sub["targets"] != float64(2) || sub["status"] != "queued" { + t.Fatalf("submit body %v", sub) + } + + // The status body is a summary: statuses, no results. + env := waitTerminal(t, s, keys["test"], id) + if env["status"] != "done" || env["targets_total"] != float64(2) || env["targets_done"] != float64(2) { + t.Fatalf("status %v", env) + } + if _, err := time.Parse(time.RFC3339, fmt.Sprint(env["created_at"])); err != nil { + t.Fatalf("created_at %v is not RFC 3339", env["created_at"]) + } + targets := env["targets"].([]any) + if len(targets) != 2 { + t.Fatalf("targets %d", len(targets)) + } + for i, x := range targets { + tg := x.(map[string]any) + if tg["idx"] != float64(i) || tg["status"] != "done" { + t.Fatalf("target %d: %v", i, tg) + } + if _, has := tg["result"]; has { + t.Fatalf("summary must not carry results: target %d has one", i) + } + } + + // One target's result: the /v1/fetch envelope. + rec := batchReq(t, s, http.MethodGet, "/v1/batch/"+id+"/1", keys["test"], nil) + if rec.Code != 200 { + t.Fatalf("GET idx: %d %s", rec.Code, rec.Body.String()) + } + one := batchDecode(t, rec) + res, _ := one["result"].(map[string]any) + if one["idx"] != float64(1) || one["status"] != "done" || res == nil { + t.Fatalf("GET idx body %v", one) + } + if c, _ := res["content"].(string); !strings.Contains(c, "hello batch /two") { + t.Fatalf("result content %q", c) + } + if res["untrusted"] != true || res["mode"] != "clean" { + t.Fatalf("result is not the /v1/fetch envelope: %v", res) + } + if src, _ := res["source"].(map[string]any); src == nil || src["final_url"] != ts.URL+"/two" { + t.Fatalf("result source %v", res["source"]) + } + + // ?results=1 inlines every result. + full := batchDecode(t, batchReq(t, s, http.MethodGet, "/v1/batch/"+id+"?results=1", keys["test"], nil)) + for i, x := range full["targets"].([]any) { + if _, has := x.(map[string]any)["result"]; !has { + t.Fatalf("results=1: target %d has no result", i) + } + } + + if got := batchReq(t, s, http.MethodGet, "/v1/batch/"+id+"/2", keys["test"], nil).Code; got != 404 { + t.Fatalf("GET idx out of range: %d, want 404", got) + } + if got := batchReq(t, s, http.MethodGet, "/v1/batch/"+id+"/x", keys["test"], nil).Code; got != 404 { + t.Fatalf("GET idx not a number: %d, want 404", got) + } +} + +func TestBatchIdempotentResubmit(t *testing.T) { + ts := pageServer(t, nil) + s, keys := newBatchServer(t, "test") + tg := map[string]any{"url": ts.URL, "mode": "clean"} + a := submit(t, s, keys["test"], tg) + b := submit(t, s, keys["test"], tg) + if a["job_id"] != b["job_id"] || b["reused"] != true { + t.Fatalf("resubmit: %v then %v", a, b) + } +} + +func TestBatchRefusedUpFront(t *testing.T) { + s, keys := newBatchServer(t, "test") + cases := []struct { + name string + body any + code int + want string + }{ + {"guard pre-check (port)", map[string]any{"targets": []any{map[string]any{"url": "http://127.0.0.1/"}, map[string]any{"url": "http://example.com:8080/"}}}, 422, "blocked"}, + {"guard pre-check (private literal)", map[string]any{"targets": []any{map[string]any{"url": "http://10.0.0.5/"}}}, 422, "blocked"}, + {"no targets", map[string]any{"targets": []any{}}, 400, ""}, + {"not json", "nope", 400, ""}, + {"empty url", map[string]any{"targets": []any{map[string]any{"url": ""}}}, 400, ""}, + {"bad mode", map[string]any{"targets": []any{map[string]any{"url": "http://127.0.0.1/", "mode": "shout"}}}, 400, ""}, + } + for _, c := range cases { + rec := batchReq(t, s, http.MethodPost, "/v1/batch", keys["test"], c.body) + if rec.Code != c.code || !strings.Contains(rec.Body.String(), c.want) { + t.Errorf("%s: %d %s, want %d containing %q", c.name, rec.Code, rec.Body.String(), c.code, c.want) + } + } + // The guard refusal names the offending index. + rec := batchReq(t, s, http.MethodPost, "/v1/batch", keys["test"], cases[0].body) + if !strings.Contains(rec.Body.String(), "1") { + t.Errorf("guard refusal does not name target 1: %s", rec.Body.String()) + } + + var many []any + for i := 0; i < 33; i++ { + many = append(many, map[string]any{"url": fmt.Sprintf("http://127.0.0.1/%d", i)}) + } + if rec := batchReq(t, s, http.MethodPost, "/v1/batch", keys["test"], map[string]any{"targets": many}); rec.Code != 422 { + t.Errorf("33 targets: %d, want 422", rec.Code) + } +} + +// 32 targets, each with a 500-rune question, fit the batch body cap even though they are well +// over the 64 KiB cap of the other endpoints. +func TestBatchBodyCap(t *testing.T) { + hold := make(chan struct{}) + defer close(hold) + ts := pageServer(t, hold) + s, keys := newBatchServer(t, "test") + q := strings.Repeat("q", 500) + var many []any + for i := 0; i < 32; i++ { + many = append(many, map[string]any{"url": fmt.Sprintf("%s/%d?pad=%s", ts.URL, i, strings.Repeat("p", 1500)), "question": q}) + } + if rec := batchReq(t, s, http.MethodPost, "/v1/batch", keys["test"], map[string]any{"targets": many}); rec.Code != 202 { + t.Fatalf("32 targets with questions (~%d KiB): %d %s", 32*2100/1024, rec.Code, rec.Body.String()) + } +} + +func TestBatchPartialFailure(t *testing.T) { + ts := pageServer(t, nil) + s, keys := newBatchServer(t, "test") + // Target 1 is a name that resolves to a private address. Names are not resolved at submit, + // so it passes the pre-check; the fetch-time guard blocks it: a per-target failure. + sub := submit(t, s, keys["test"], map[string]any{"url": ts.URL}, map[string]any{"url": "http://internal.test/"}) + env := waitTerminal(t, s, keys["test"], sub["job_id"].(string)) + if env["status"] != "partial" { + t.Fatalf("status %v", env["status"]) + } + second := env["targets"].([]any)[1].(map[string]any) + if second["status"] != "failed" { + t.Fatalf("target 1: %v", second) + } + if e, _ := second["error"].(string); !strings.Contains(e, "blocked") { + t.Fatalf("target 1 error %q", e) + } +} + +func TestBatchOwnership404(t *testing.T) { + hold := make(chan struct{}) + defer close(hold) + ts := pageServer(t, hold) + s, keys := newBatchServer(t, "test", "other") + id := submit(t, s, keys["test"], map[string]any{"url": ts.URL})["job_id"].(string) + for _, p := range []string{"/v1/batch/" + id, "/v1/batch/" + id + "/0"} { + if got := batchReq(t, s, http.MethodGet, p, keys["other"], nil).Code; got != 404 { + t.Fatalf("other caller GET %s: %d (must not reveal the job)", p, got) + } + } + if got := batchReq(t, s, http.MethodDelete, "/v1/batch/"+id, keys["other"], nil).Code; got != 404 { + t.Fatalf("other caller DELETE: %d", got) + } + if got := batchReq(t, s, http.MethodGet, "/v1/batch/"+id, keys["test"], nil).Code; got != 200 { + t.Fatalf("owner GET: %d", got) + } + if got := batchReq(t, s, http.MethodGet, "/v1/batch/no-such-job", keys["test"], nil).Code; got != 404 { + t.Fatalf("unknown job: %d", got) + } + if got := batchReq(t, s, http.MethodGet, "/v1/batch/"+id, "not-a-key", nil).Code; got != 401 { + t.Fatalf("bad key: %d", got) + } +} + +func TestBatchCancel(t *testing.T) { + hold := make(chan struct{}) + ts := pageServer(t, hold) + s, keys := newBatchServer(t, "test") + id := submit(t, s, keys["test"], map[string]any{"url": ts.URL + "/a"}, map[string]any{"url": ts.URL + "/b"})["job_id"].(string) + rec := batchReq(t, s, http.MethodDelete, "/v1/batch/"+id, keys["test"], nil) + if rec.Code != 200 { + t.Fatalf("cancel: %d %s", rec.Code, rec.Body.String()) + } + if b := batchDecode(t, rec); b["success"] != true || b["status"] != "cancelled" { + t.Fatalf("cancel body %v", b) + } + close(hold) + env := waitTerminal(t, s, keys["test"], id) + if env["status"] != "cancelled" { + t.Fatalf("status %v", env["status"]) + } + if tg := env["targets"].([]any)[1].(map[string]any); tg["status"] != "pending" { + t.Fatalf("target 1 after cancel: %v", tg) + } + if got := batchReq(t, s, http.MethodDelete, "/v1/batch/"+id, keys["test"], nil).Code; got != 409 { + t.Fatalf("cancelling a finished job: %d, want 409", got) + } +} + +// The request metrics never get a job id as a label value. +func TestBatchMetricsPathLabel(t *testing.T) { + ts := pageServer(t, nil) + s, keys := newBatchServer(t, "test") + id := submit(t, s, keys["test"], map[string]any{"url": ts.URL})["job_id"].(string) + waitTerminal(t, s, keys["test"], id) + batchReq(t, s, http.MethodGet, "/v1/batch/"+id+"/0", keys["test"], nil) + if strings.Contains(promText(t, s), id) { + t.Fatal("a job id appears in /metrics") + } +} diff --git a/internal/api/batchrun_test.go b/internal/api/batchrun_test.go new file mode 100644 index 0000000..5e5c14c --- /dev/null +++ b/internal/api/batchrun_test.go @@ -0,0 +1,202 @@ +package api + +// P6 task 06: the runner wired to fetchTarget and the metrics, with no HTTP yet. The helpers +// here (newBatchServer, pageServer, fakeDNS) are shared with task 07's batch_test.go. + +import ( + "context" + "errors" + "fmt" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "net/netip" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "git.wntrmute.dev/kyle/sift/internal/auth" + "git.wntrmute.dev/kyle/sift/internal/batch" + "git.wntrmute.dev/kyle/sift/internal/dcache" + "git.wntrmute.dev/kyle/sift/internal/fetch" + "git.wntrmute.dev/kyle/sift/internal/guard" +) + +var loopback = []netip.Prefix{netip.MustParsePrefix("127.0.0.0/8")} + +// fakeDNS resolves internal.test to a private address; every other name fails to resolve. +func fakeDNS(_ context.Context, host string) ([]netip.Addr, error) { + if host == "internal.test" { + return []netip.Addr{netip.MustParseAddr("10.0.0.5")}, nil + } + return nil, fmt.Errorf("no such host %s", host) +} + +// newBatchServer builds a Server with the batch runner wired over an in-memory dcache through +// s.NewBatchRunner, exactly as serve does. callers are the names to provision (each gets a key). +func newBatchServer(t *testing.T, callers ...string) (*Server, map[string]string) { + t.Helper() + dir := t.TempDir() + keysPath := filepath.Join(dir, "keys.toml") + keys := map[string]string{} + for _, name := range callers { + out := filepath.Join(dir, name) + if err := auth.Add(keysPath, name, "hermes-fleet", out); err != nil { + t.Fatal(err) + } + b, err := os.ReadFile(out) + if err != nil { + t.Fatal(err) + } + keys[name] = strings.TrimSpace(string(b)) + } + store, err := auth.Load(keysPath) + if err != nil { + t.Fatal(err) + } + g := &guard.Guard{AllowPrefixes: loopback, LookupIP: fakeDNS} + f := &fetch.Fetcher{Guard: g, UA: "sift-test"} + log := slog.New(slog.NewTextHandler(io.Discard, nil)) + s := New("test", g, f, store, log) + db, err := dcache.Open("", 1<<20, log) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = db.Close() }) + s.Batch = s.NewBatchRunner(db, batch.Config{}) + t.Cleanup(s.Batch.Close) // before db.Close + return s, keys +} + +// pageServer serves a small HTML page at every path; hold, when non-nil, blocks each request +// until it is closed. +func pageServer(t *testing.T, hold chan struct{}) *httptest.Server { + t.Helper() + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if hold != nil { + select { + case <-hold: + case <-r.Context().Done(): + return + } + } + w.Header().Set("Content-Type", "text/html; charset=utf-8") + fmt.Fprintf(w, `T

hello batch %s

`, r.URL.Path) + })) + t.Cleanup(ts.Close) + return ts +} + +func waitJob(t *testing.T, r *batch.Runner, id string) batch.Job { + t.Helper() + deadline := time.Now().Add(10 * time.Second) + for time.Now().Before(deadline) { + j, _, ok := r.Status(id) + if ok && (j.Status == "done" || j.Status == "partial" || j.Status == "failed" || j.Status == "cancelled") { + return j + } + time.Sleep(5 * time.Millisecond) + } + t.Fatal("job never finished") + return batch.Job{} +} + +func promText(t *testing.T, s *Server) string { + t.Helper() + req := httptest.NewRequest(http.MethodGet, "/metrics", nil) + req.RemoteAddr, req.Host = "127.0.0.1:9", "127.0.0.1:8890" + rec := httptest.NewRecorder() + s.ServeHTTP(rec, req) + return rec.Body.String() +} + +// The runner fetches through fetchTarget as the submitting caller; a name that resolves to a +// private address is a per-target "blocked" failure; the hooks reach the metrics. +func TestBatchRunnerWired(t *testing.T) { + ts := pageServer(t, nil) + s, _ := newBatchServer(t, "test") + c := &auth.Caller{Name: "test", Workspace: "hermes-fleet"} + id, reused, err := s.Batch.Submit(context.Background(), c, dcache.ClassFleet, []batch.Target{ + {URL: ts.URL + "/one"}, {URL: "http://internal.test/"}, + }) + if err != nil || reused { + t.Fatalf("submit: %v %v", err, reused) + } + if j := waitJob(t, s.Batch, id); j.Status != "partial" { + t.Fatalf("job %+v", j) + } + _, ts2, _ := s.Batch.Status(id) + if ts2[0].Status != "done" || ts2[0].Result == nil || !strings.Contains(ts2[0].Result.Content, "hello batch /one") { + t.Fatalf("target 0: %+v", ts2[0]) + } + if ts2[0].Result.Meta["mode"] != "clean" || ts2[0].Result.Meta["untrusted"] != true { + t.Fatalf("target 0 meta %v: not the fetchTarget envelope", ts2[0].Result.Meta) + } + if ts2[1].Status != "failed" || !strings.Contains(ts2[1].Error, "blocked") { + t.Fatalf("target 1: %+v", ts2[1]) + } + out := promText(t, s) + for _, want := range []string{ + `sift_batch_jobs_total{outcome="created"} 1`, + `sift_batch_jobs_total{outcome="partial"} 1`, + `sift_batch_targets_total{outcome="ok"} 1`, + `sift_batch_targets_total{outcome="blocked"} 1`, + `sift_batch_result_bytes_count 1`, + `sift_batch_queue_depth 0`, + } { + if !strings.Contains(out, want) { + t.Errorf("metrics lack %q", want) + } + } +} + +// The adapter turns fetch errors into what the runner and the metrics need. +func TestBatchOutcomeClasses(t *testing.T) { + cases := []struct { + err error + want string + }{ + {nil, "ok"}, + {fmt.Errorf("busy: %w", batch.ErrRetryLater), "busy"}, + {&batch.TargetError{Status: 403, Msg: "blocked: private address"}, "blocked"}, + {&batch.TargetError{Status: 429, Msg: "busy"}, "busy"}, + {&batch.TargetError{Status: 504, Msg: "timeout"}, "timeout"}, + {&batch.TargetError{Status: 415, Msg: "unsupported content type"}, "unsupported"}, + {&batch.TargetError{Status: 422, Msg: "document too long for question (6001 chunks); use grep or pages"}, "too_long"}, + {&batch.TargetError{Status: 422, Msg: "unreadable PDF"}, "upstream_4xx"}, + {&batch.TargetError{Status: 404, Msg: "not found"}, "upstream_4xx"}, + {&batch.TargetError{Status: 500, Msg: "internal error"}, "upstream_5xx"}, + {errors.New("something else"), "upstream_5xx"}, + } + for _, c := range cases { + if got := batchOutcome(c.err); got != c.want { + t.Errorf("batchOutcome(%v) = %q, want %q", c.err, got, c.want) + } + } +} + +// batchFetch maps a busy fetch to ErrRetryLater (the runner retries it) and every other +// failure to a TargetError with the /v1/fetch status. +func TestBatchFetchErrors(t *testing.T) { + s, _ := newBatchServer(t, "test") + c := &auth.Caller{Name: "test", Workspace: "hermes-fleet"} + _, err := s.batchFetch(context.Background(), c, batch.Target{URL: "http://internal.test/"}) + var te *batch.TargetError + if !errors.As(err, &te) || te.Status != 403 || !strings.Contains(te.Msg, "blocked") { + t.Fatalf("private name: %v, want a TargetError 403 blocked", err) + } + // Fill the global fetch gate so the next fetch is refused busy. + for i := 0; i < 8; i++ { + if err := s.acquire(fmt.Sprintf("filler%d", i), "http://filler.test/"); err != nil { + t.Fatal(err) + } + } + ts := pageServer(t, nil) + _, err = s.batchFetch(context.Background(), c, batch.Target{URL: ts.URL}) + if !errors.Is(err, batch.ErrRetryLater) { + t.Fatalf("busy gate: %v, want an error wrapping batch.ErrRetryLater", err) + } +} diff --git a/internal/api/fetchtarget_test.go b/internal/api/fetchtarget_test.go new file mode 100644 index 0000000..e512ad2 --- /dev/null +++ b/internal/api/fetchtarget_test.go @@ -0,0 +1,94 @@ +package api + +// P6 task 05: v1fetch's per-target pipeline is extracted into s.fetchTarget, which the batch +// runner also calls. These tests pin that the two agree; every existing v1fetch test must also +// pass unchanged. + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "net/netip" + "strings" + "testing" + + "git.wntrmute.dev/kyle/sift/internal/auth" + "git.wntrmute.dev/kyle/sift/internal/batch" + "git.wntrmute.dev/kyle/sift/internal/fetch" + "git.wntrmute.dev/kyle/sift/internal/guard" +) + +// newTargetServer is setupAPI with loopback allowed; its key belongs to caller "test". +func newTargetServer(t *testing.T) (*Server, string) { + t.Helper() + return setupAPI(t, &guard.Guard{AllowPrefixes: []netip.Prefix{netip.MustParsePrefix("127.0.0.0/8")}}) +} + +func TestFetchTargetMatchesV1Fetch(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "text/html; charset=utf-8") + _, _ = w.Write([]byte(`Same

Same

one pipeline, two callers

`)) + })) + defer ts.Close() + s, key := newTargetServer(t) + c := &auth.Caller{Name: "test", Workspace: "hermes-fleet"} + + res, err := s.fetchTarget(context.Background(), c, batch.Target{URL: ts.URL}) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(res.Content, "one pipeline, two callers") { + t.Fatalf("content %q", res.Content) + } + if _, has := res.Meta["content"]; has { + t.Fatal("Meta must not repeat the content") + } + for _, k := range []string{"mode", "untrusted", "source", "warnings", "sizes", "timings"} { + if _, has := res.Meta[k]; !has { + t.Errorf("Meta lacks %q (it carries every envelope field except content)", k) + } + } + if res.Meta["mode"] != "clean" || res.Meta["untrusted"] != true { + t.Fatalf("meta %v", res.Meta) + } + + // The same URL through /v1/fetch: identical content and mode, and the envelope has exactly + // the fields of Meta plus content. + rec := postJSON(t, s, "/v1/fetch", key, map[string]any{"url": ts.URL}) + if rec.Code != 200 { + t.Fatalf("v1fetch %d %s", rec.Code, rec.Body.String()) + } + var env map[string]any + if err := json.Unmarshal(rec.Body.Bytes(), &env); err != nil { + t.Fatal(err) + } + if env["content"] != res.Content || env["mode"] != res.Meta["mode"] { + t.Fatalf("v1fetch and fetchTarget disagree:\n%v\n%+v", env, res) + } + if len(env) != len(res.Meta)+1 { + t.Fatalf("envelope keys %d, Meta keys %d: want Meta + content", len(env), len(res.Meta)) + } +} + +func TestFetchTargetErrors(t *testing.T) { + s, _ := newTargetServer(t) + c := &auth.Caller{Name: "test", Workspace: "hermes-fleet"} + cases := []struct { + name string + target batch.Target + status int + msg string + }{ + {"blocked", batch.Target{URL: "http://10.0.0.5/"}, 403, "blocked"}, + {"bad mode", batch.Target{URL: "http://127.0.0.1/", Options: batch.Options{Mode: "shout"}}, 400, ""}, + } + for _, tc := range cases { + _, err := s.fetchTarget(context.Background(), c, tc.target) + var fe *fetch.Error + if !errors.As(err, &fe) || fe.Status != tc.status || !strings.Contains(fe.Msg, tc.msg) { + t.Errorf("%s: err %v, want a *fetch.Error %d containing %q", tc.name, err, tc.status, tc.msg) + } + } +} diff --git a/internal/api/pdf.go b/internal/api/pdf.go index d13e313..c141247 100644 --- a/internal/api/pdf.go +++ b/internal/api/pdf.go @@ -58,7 +58,11 @@ func (s *Server) extractPDF(ctx context.Context, c *auth.Caller, res *fetch.Reso return v, err } start := time.Now() - v, err := s.pool().Do(ctx, s.callerClass(c)+"|"+res.SHA256(), c.Name, fn) + caller := c.Name + if isBatchFetch(ctx) { + caller = c.Name + "/batch" + } + v, err := s.pool().Do(ctx, s.callerClass(c)+"|"+res.SHA256(), caller, fn) total := time.Since(start) // The job's own extraction time is what this call measured it to be; the // rest of Do is time spent waiting for a slot. diff --git a/internal/api/target.go b/internal/api/target.go new file mode 100644 index 0000000..415895e --- /dev/null +++ b/internal/api/target.go @@ -0,0 +1,232 @@ +package api + +import ( + "context" + "errors" + "fmt" + "time" + "unicode/utf8" + + "git.wntrmute.dev/kyle/sift/internal/auth" + "git.wntrmute.dev/kyle/sift/internal/batch" + "git.wntrmute.dev/kyle/sift/internal/clean" + "git.wntrmute.dev/kyle/sift/internal/digest" + "git.wntrmute.dev/kyle/sift/internal/fetch" + "git.wntrmute.dev/kyle/sift/internal/fleetctx" + "git.wntrmute.dev/kyle/sift/internal/pdfdoc" + "git.wntrmute.dev/kyle/sift/internal/recover" +) + +// fetchTarget runs one target's per-target pipeline: fetch → recover → PDF render or clean → +// question limit → digest → context → log line → envelope. It is the single body both /v1/fetch +// and the batch worker drive, so it must not change what v1fetch returned. +func (s *Server) fetchTarget(ctx context.Context, c *auth.Caller, t batch.Target) (batch.Result, error) { + q := t.Options.Question + if len([]rune(q)) > 500 { + rns := []rune(q) + q = string(rns[:500]) + } + mode := t.Options.Mode + if mode == "" { + if q != "" { + mode = "digest" + } else { + mode = "clean" + } + } + if mode != "clean" && mode != "raw" && mode != "digest" { + return batch.Result{}, &fetch.Error{Status: 400, Msg: "invalid url"} + } + wantCtx := q != "" + if t.Options.Context != nil { + wantCtx = *t.Options.Context + } + + t0 := time.Now() + onlyMain := true + res, cl, doc, ferr := s.doFetch(ctx, c, t.URL, t.Options.JQ, onlyMain) + var snap *recover.Result + if ferr != nil && t.Options.Recover && recoverWorthy(ferr) { + sr, rerr := recover.Try(ctx, s.Fetch, t.URL) + if rerr == nil && sr != nil && sr.Res != nil { + res, cl, doc, ferr = s.cleanFetched(sr.Res, t.Options.JQ, onlyMain) + snap = sr + } + } + if ferr != nil { + return batch.Result{}, ferr + } + fetchMS := time.Since(t0).Milliseconds() + + if doc == nil && (t.Options.Pages != "" || t.Options.Grep != "") { + return batch.Result{}, &fetch.Error{Status: 400, Msg: "pages and grep apply to PDFs only"} + } + + var docLen, pdfPages int + var content string + var pdfMap map[string]any + if doc != nil { + pdfPages = doc.Pages + if mode == "raw" { + return batch.Result{}, &fetch.Error{Status: 400, Msg: "raw mode does not apply to PDFs"} + } + rp, err := s.renderPDF(doc, pdfdoc.Options{Pages: t.Options.Pages, Grep: t.Options.Grep, OutCap: s.OutCap}) + if err != nil { + if errors.Is(err, pdfdoc.ErrBadPages) || errors.Is(err, pdfdoc.ErrBadGrep) { + return batch.Result{}, &fetch.Error{Status: 400, Msg: err.Error()} + } + return batch.Result{}, err + } + full := pdfdoc.Full(doc) + docLen = utf8.RuneCountInString(full) + if mode == "digest" { + content = full + } else { + content = rp.Content + } + pdfMap = map[string]any{ + "pages": doc.Pages, + "title": doc.Title, + "outline": doc.Outline, + "extracted": len(doc.Text), + "first": rp.First, + "last": rp.Last, + "map": rp.Map, + "grep_hits": rp.GrepHits, + "note": rp.Note, + } + } else { + content = cl.Markdown + if cl.Kind == "json" { + content = cl.HTML + } + if mode == "raw" { + content = string(res.Body) + if len(content) > s.OutCap { + content = clean.TruncateUTF8(content, s.OutCap) + cl.Trunc = true + cl.Warning = "truncated to output cap" + } + } + } + warn := []string{} + if cl.Warning != "" { + warn = append(warn, cl.Warning) + } + + var digMeta map[string]any + var digestMS int64 + if mode == "digest" { + if doc != nil { + n := len(digest.Split(pdfdoc.Full(doc), digest.ChunkTokens)) + limit := s.PDFMaxQuestionChunks + if limit == 0 { + limit = DefaultPDFMaxQuestionChunks + } + if n > limit { + return batch.Result{}, &fetch.Error{Status: 422, Msg: fmt.Sprintf("document too long for question (%d chunks); use grep or pages", n)} + } + } + if s.Digest == nil { + warn = append(warn, "digest fell back to clean: unset") + mode = "clean" + content, warn = capDigestFallback(content, warn) + digMeta = map[string]any{"fallback": "unset"} + } else { + dr := s.Digest.Run(ctx, content, q, res.FinalURL, s.callerClass(c)) + digestMS = dr.DigestMS + if !dr.Cached { + s.Prom.ObserveDigest(time.Duration(dr.DigestMS) * time.Millisecond) + } + if dr.Fallback != "" { + warn = append(warn, "digest fell back to clean: "+dr.Fallback) + mode = "clean" + content, warn = capDigestFallback(content, warn) + digMeta = map[string]any{"fallback": dr.Fallback, "model": dr.Model} + } else { + content = dr.Text + digMeta = map[string]any{ + "model": dr.Model, + "chunks_used": dr.ChunksUsed, + "chunks_total": dr.ChunksTotal, + "retrieval": dr.Retrieval, + "embedded": dr.Embedded, + "fallback": false, + "cached": dr.Cached, + } + } + } + } + + var ctxItems []fleetctx.Item + var ctxMS int64 + if wantCtx && s.Ctx != nil { + ct0 := time.Now() + query := fleetctx.Query(q, cl.Title, headings(cl.Markdown)) + items, cw := s.Ctx.Search(ctx, c.Workspace, c.Name, query) + ctxMS = time.Since(ct0).Milliseconds() + if cw != "" { + warn = append(warn, cw) + } + ctxItems = items + } + + qLen := len(q) + if qLen > 80 { + qLen = 80 + } + fb, _ := "", false + if digMeta != nil { + if s, ok := digMeta["fallback"].(string); ok { + fb = s + } + } + s.Log.Info("fetch", "caller", c.Name, "host", s.logHost(c, res.FinalURL), "mode", mode, + "q_len", qLen, "raw_tokens", len(res.Body)/4, "returned_tokens", len(content)/4, + "pdf_pages", pdfPages, "digest_ms", digestMS, "fallback", fb) + + src := map[string]any{ + "url": res.URL, + "final_url": res.FinalURL, + "status": res.Status, + "content_type": res.ContentType, + "fetched_at": res.FetchedAt.Format(time.RFC3339), + "hash": res.SHA256(), + "bytes": len(res.Body), + "cached": len(res.Body) == 0, + "kind": "live", + } + if snap != nil { + src["kind"] = "snapshot" + src["via"] = string(snap.Via) + src["original_url"] = snap.OriginalURL + if snap.SnapshotAt != "" { + src["snapshot_at"] = snap.SnapshotAt + } else { + src["snapshot_at"] = nil + warn = append(warn, "snapshot date unknown") + } + } + m := map[string]any{ + "mode": mode, + "untrusted": true, + "source": src, + "warnings": warn, + "sizes": map[string]int{ + "raw_tokens": docLen / 4, + "clean_tokens": digest.EstTokens(cl.Markdown), + "returned_tokens": digest.EstTokens(content), + }, + "timings": map[string]int64{"fetch_ms": fetchMS, "digest_ms": digestMS, "context_ms": ctxMS}, + } + if pdfMap != nil { + m["pdf"] = pdfMap + } + if digMeta != nil { + m["digest"] = digMeta + } + if wantCtx { + m["context"] = ctxItems + } + return batch.Result{Content: content, Meta: m}, nil +} diff --git a/internal/batch/batch.go b/internal/batch/batch.go new file mode 100644 index 0000000..49677b0 --- /dev/null +++ b/internal/batch/batch.go @@ -0,0 +1,394 @@ +// Package batch is the batch runner core: it owns job lifecycle, idempotency and +// the per-caller / global caps. It knows nothing about HTTP, the guard or the real +// fetch pipeline — per-target work is an injected FetchFunc, and persistence is the +// dcache store. Jobs never resume after a restart, so the caller is held in memory, +// never written to the store. +package batch + +import ( + "context" + "crypto/rand" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "log/slog" + "sync" + "time" + + "git.wntrmute.dev/kyle/sift/internal/auth" + "git.wntrmute.dev/kyle/sift/internal/dcache" +) + +type Options struct { + Mode string `json:"mode,omitempty"` + Question string `json:"question,omitempty"` + Pages string `json:"pages,omitempty"` + Grep string `json:"grep,omitempty"` + JQ string `json:"jq,omitempty"` + Context *bool `json:"context,omitempty"` // nil = /v1/fetch's default (on when there is a question) + Recover bool `json:"recover,omitempty"` +} + +type Target struct { + URL string `json:"url"` + Options Options `json:"options"` +} + +// Result is a finished target: the /v1/fetch envelope's content, and every other envelope field +// in Meta. It is stored JSON-encoded in the target's result column. +type Result struct { + Content string `json:"content"` + Meta map[string]any `json:"meta,omitempty"` +} + +// TargetError is a failure with an agent-facing hint. Status is the HTTP status the same +// failure gets from /v1/fetch; the runner ignores it (the metrics hook reads it). Any other +// error is stored with its Error() text and no hint. +type TargetError struct { + Status int + Msg, Hint string +} + +func (e *TargetError) Error() string { return e.Msg } + +// FetchFunc does one target for the caller that submitted the job. +type FetchFunc func(ctx context.Context, c *auth.Caller, t Target) (Result, error) + +type Job struct { + ID string + Caller string + Status string // queued | running | done | partial | failed | cancelled + Error string + TargetsTotal int + TargetsDone int + CreatedAt int64 // unix seconds +} + +type TargetStatus struct { + Idx int + URL string + Status string // pending | running | done | failed + Result *Result + Error string + Hint string +} + +// Hooks observe the runner for metrics. Any nil field is skipped. +type Hooks struct { + Job func(outcome string) // created, reused, done, partial, failed, cancelled + Target func(err error, d time.Duration, resultBytes int) // once per stored target; err nil on success + JobSeconds func(d time.Duration) // submit → terminal, once per job + Depth func(live int) // live jobs, after every change + Inflight func(n int) // targets being fetched, after every change +} + +type Config struct { + TTL time.Duration // 0 → 24h + TargetsPerJob int // 0 → 32 + JobsPerCaller int // 0 → 1 + JobsTotal int // 0 → 8 + Workers int // 0 → 1; values above 2 are clamped to 2 + MaxBytes int64 // 0 → 64 MiB + RetryWait time.Duration // 0 → 2s: the pause before a busy target is tried again + RetryFor time.Duration // 0 → 10m: how long one target may keep coming back busy + Hooks Hooks +} + +var ( + ErrNoTargets = errors.New("no targets") + ErrTooManyTargets = errors.New("too many targets") + ErrCallerBusy = errors.New("busy") + ErrBusy = errors.New("jobs busy") + ErrRetryLater = errors.New("retry later") // a FetchFunc wraps this when the target was only busy +) + +// terminal statuses mark a job as finished; only a queued job can be cancelled. +func isJobTerminal(status string) bool { + switch status { + case "done", "partial", "failed", "cancelled": + return true + } + return false +} + +type Runner struct { + db *dcache.DB + fetch FetchFunc + log *slog.Logger + cfg Config + closeOnce sync.Once + submitMu sync.Mutex // serialises Submit: idempotency lookup, caps and create are one step + + mu sync.Mutex // guards everything below + cond *sync.Cond // on mu + live []*liveJob // the rotation, in submit order + cursor int // where the next scan starts, within [0, len(live)]; read as cursor % len(live) + closed bool + ctx context.Context // the FetchFunc context; Close cancels it + stop context.CancelFunc + wg sync.WaitGroup // the workers + inflight int +} + +func New(db *dcache.DB, fetch FetchFunc, cfg Config, log *slog.Logger) *Runner { + // Zero or negative means the default: a negative TTL or byte budget would make the sweep or + // the eviction delete everything, and negative workers would panic the WaitGroup. + if cfg.TTL <= 0 { + cfg.TTL = 24 * time.Hour + } + if cfg.TargetsPerJob <= 0 { + cfg.TargetsPerJob = 32 + } + if cfg.JobsPerCaller <= 0 { + cfg.JobsPerCaller = 1 + } + if cfg.JobsTotal <= 0 { + cfg.JobsTotal = 8 + } + if cfg.Workers <= 0 { + cfg.Workers = 1 + } + if cfg.Workers > 2 { + cfg.Workers = 2 + } + if cfg.MaxBytes <= 0 { + cfg.MaxBytes = 64 << 20 + } + if cfg.RetryWait <= 0 { + cfg.RetryWait = 2 * time.Second + } + if cfg.RetryFor <= 0 { + cfg.RetryFor = 10 * time.Minute + } + ctx, stop := context.WithCancel(context.Background()) + r := &Runner{ + db: db, + fetch: fetch, + log: log, + cfg: cfg, + ctx: ctx, + stop: stop, + } + r.cond = sync.NewCond(&r.mu) + r.wg.Add(cfg.Workers) + for i := 0; i < cfg.Workers; i++ { + go r.worker() + } + return r +} + +// Submit stores the job and its targets before it returns. The order matters: an empty +// request, then idempotency (which never touches the caps), then the caps, then the write. +func (r *Runner) Submit(ctx context.Context, c *auth.Caller, callerClass string, targets []Target) (id string, reused bool, err error) { + if len(targets) == 0 { + return "", false, ErrNoTargets + } + + raw, err := json.Marshal(targets) + if err != nil { + return "", false, fmt.Errorf("batch: marshal targets: %w", err) + } + sum := sha256.Sum256([]byte(c.Name + "\x00" + string(raw))) + idemKey := hex.EncodeToString(sum[:]) + + // The lookup, the caps and the create must be one step, or concurrent submits all pass the + // checks before any of them is stored. + r.submitMu.Lock() + defer r.submitMu.Unlock() + + now := time.Now().Unix() + if existing, ok := r.db.BatchJobByIDem(c.Name, idemKey, now, r.cfg.TTL); ok { + r.callJob("reused") + return existing, true, nil + } + + if len(targets) > r.cfg.TargetsPerJob { + return "", false, ErrTooManyTargets + } + if r.db.LiveBatchJobs(c.Name) >= r.cfg.JobsPerCaller { + return "", false, ErrCallerBusy + } + if r.db.LiveBatchJobsTotal() >= r.cfg.JobsTotal { + return "", false, ErrBusy + } + + id, err = randomID() + if err != nil { + return "", false, fmt.Errorf("batch: job id: %w", err) + } + + if err := r.db.CreateBatchJob(dcache.BatchJob{ + ID: id, + Caller: c.Name, + CallerClass: callerClass, + IDemKey: idemKey, + Status: "queued", + TargetsTotal: len(targets), + CreatedAt: now, + Lease: now, + }); err != nil { + r.log.Warn("batch: create job", "job", id, "err", err) + return "", false, fmt.Errorf("batch: create job: %w", err) + } + + btargets := make([]dcache.BatchTarget, len(targets)) + for i := range targets { + opts, _ := json.Marshal(targets[i].Options) + btargets[i] = dcache.BatchTarget{ + JobID: id, + Idx: i, + URL: targets[i].URL, + Options: string(opts), + Status: "pending", + } + } + if err := r.db.PutBatchTargets(id, btargets); err != nil { + r.log.Warn("batch: store targets", "job", id, "err", err) + // Never leave a queued job that no worker will run: it would hold the caller's cap. + _ = r.db.SetBatchJobStatus(id, "failed", "store", now) + return "", false, fmt.Errorf("batch: store targets: %w", err) + } + + r.mu.Lock() + r.live = append(r.live, &liveJob{ + id: id, + caller: c, + targets: targets, + created: time.Unix(now, 0), + }) + r.callDepth(len(r.live)) + r.cond.Broadcast() + r.mu.Unlock() + + r.callJob("created") + r.db.SweepBatch(now, r.cfg.TTL) + return id, false, nil +} + +// Cancel cancels a job. A job still in the rotation (queued or with a target in flight) is marked +// cancelled at once: its in-flight target, if any, finishes and is stored without overwriting +// cancelled, and no further target starts. A job not in the rotation (unknown or already terminal) +// keeps task 02's behaviour. +func (r *Runner) Cancel(id string) bool { + r.mu.Lock() + var j *liveJob + for _, lj := range r.live { + if lj.id == id { + j = lj + break + } + } + if j != nil { + if j.cancelled { // a concurrent DELETE got here first + r.mu.Unlock() + return false + } + j.cancelled = true + if err := r.db.SetBatchJobStatus(id, "cancelled", "", time.Now().Unix()); err != nil { + r.log.Warn("batch: cancel job", "job", id, "err", err) + } + r.callJob("cancelled") + if !j.running { + r.removeLocked(j) + r.callDepth(len(r.live)) + } + r.cond.Broadcast() + r.mu.Unlock() + return true + } + r.mu.Unlock() + + jb, ok := r.db.BatchJob(id) + if !ok || isJobTerminal(jb.Status) { + return false + } + if err := r.db.SetBatchJobStatus(id, "cancelled", "", time.Now().Unix()); err != nil { + r.log.Warn("batch: cancel job", "job", id, "err", err) + return false + } + r.callJob("cancelled") + return true +} + +// Status maps the store rows for a job and its targets; Result is decoded from the result +// column. It writes nothing. +func (r *Runner) Status(id string) (Job, []TargetStatus, bool) { + jb, ok := r.db.BatchJob(id) + if !ok { + return Job{}, nil, false + } + j := Job{ + ID: jb.ID, + Caller: jb.Caller, + Status: jb.Status, + Error: jb.Error, + TargetsTotal: jb.TargetsTotal, + TargetsDone: jb.TargetsDone, + CreatedAt: jb.CreatedAt, + } + tbs, _ := r.db.BatchTargets(id) + ts := make([]TargetStatus, 0, len(tbs)) + for _, bt := range tbs { + var res *Result + if bt.Result != nil { + res = new(Result) + if err := json.Unmarshal([]byte(*bt.Result), res); err != nil { + r.log.Warn("batch: decode result", "job", id, "err", err) + } + } + ts = append(ts, TargetStatus{ + Idx: bt.Idx, + URL: bt.URL, + Status: bt.Status, + Result: res, + Error: strOrEmpty(bt.Error), + Hint: strOrEmpty(bt.Hint), + }) + } + return j, ts, true +} + +// Touch bumps the job's lease to now so a slow owner does not lose it. +func (r *Runner) Touch(id string) { + now := time.Now().Unix() + if err := r.db.TouchBatchJob(id, now); err != nil { + r.log.Warn("batch: touch job", "job", id, "err", err) + } +} + +// Close stops the workers, cancels their context, and waits for an in-flight fetch to return. +// Safe to call more than once. Jobs still live stay queued or running in the store, for +// MarkInFlightFailed("restart") on the next start. +func (r *Runner) Close() { + r.closeOnce.Do(func() { + r.mu.Lock() + r.closed = true + r.stop() + r.cond.Broadcast() + r.mu.Unlock() + r.wg.Wait() + }) +} + +func (r *Runner) callJob(outcome string) { + if r.cfg.Hooks.Job != nil { + r.cfg.Hooks.Job(outcome) + } +} + +func randomID() (string, error) { + var b [16]byte + if _, err := rand.Read(b[:]); err != nil { + return "", err + } + return hex.EncodeToString(b[:]), nil +} + +func strOrEmpty(s *string) string { + if s == nil { + return "" + } + return *s +} diff --git a/internal/batch/review_test.go b/internal/batch/review_test.go new file mode 100644 index 0000000..2712e3a --- /dev/null +++ b/internal/batch/review_test.go @@ -0,0 +1,177 @@ +package batch + +// P6 owner review: regression tests for the runner (findings from the independent review). + +import ( + "context" + "fmt" + "sync" + "sync/atomic" + "testing" + "time" + + "git.wntrmute.dev/kyle/sift/internal/auth" + "git.wntrmute.dev/kyle/sift/internal/dcache" +) + +// A job cancelled while its in-flight target comes back busy leaves the rotation. +func TestCancelWhileBusyLeavesRotation(t *testing.T) { + started := make(chan struct{}, 1) + release := make(chan struct{}) + var depth atomic.Int64 + fetch := func(context.Context, *auth.Caller, Target) (Result, error) { + select { + case started <- struct{}{}: + default: + } + <-release + return Result{}, fmt.Errorf("busy: %w", ErrRetryLater) + } + r := newRunner(t, fetch, Config{RetryWait: time.Millisecond, Hooks: Hooks{Depth: func(n int) { depth.Store(int64(n)) }}}) + id, _, err := r.Submit(context.Background(), alice, dcache.ClassFleet, urls("a", "b")) + if err != nil { + t.Fatal(err) + } + <-started + if !r.Cancel(id) { + t.Fatal("cancel returned false") + } + close(release) + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + r.mu.Lock() + n := len(r.live) + r.mu.Unlock() + if n == 0 && depth.Load() == 0 { + return + } + time.Sleep(5 * time.Millisecond) + } + t.Fatalf("cancelled job still in the rotation (depth %d)", depth.Load()) +} + +// Close stops the workers: no further target is fetched, and the job is left live in the +// store for MarkInFlightFailed("restart") to reap, not failed target by target. +func TestCloseStopsDraining(t *testing.T) { + started := make(chan struct{}, 1) + var calls atomic.Int64 + fetch := func(ctx context.Context, _ *auth.Caller, _ Target) (Result, error) { + calls.Add(1) + select { + case started <- struct{}{}: + default: + } + <-ctx.Done() + return Result{}, ctx.Err() + } + r := newRunner(t, fetch, Config{}) + id, _, err := r.Submit(context.Background(), alice, dcache.ClassFleet, urls("a", "b", "c", "d")) + if err != nil { + t.Fatal(err) + } + <-started + r.Close() + if n := calls.Load(); n != 1 { + t.Fatalf("%d fetches after Close, want only the one in flight", n) + } + j, ts, _ := r.Status(id) + if j.Status != "running" && j.Status != "queued" { + t.Fatalf("job %q after Close, want it left live for the restart reaper", j.Status) + } + for _, x := range ts { + if x.Status != "pending" { + t.Fatalf("target %d is %q after Close, want pending", x.Idx, x.Status) + } + } +} + +// Concurrent submits respect the caps and idempotency: one job per caller, and identical +// lists all get the same job. +func TestConcurrentSubmits(t *testing.T) { + release := make(chan struct{}) + defer close(release) + fetch := func(context.Context, *auth.Caller, Target) (Result, error) { + <-release + return Result{Content: "ok"}, nil + } + r := newRunner(t, fetch, Config{}) + var wg sync.WaitGroup + var mu sync.Mutex + ids := map[string]bool{} + for i := 0; i < 20; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + id, _, err := r.Submit(context.Background(), alice, dcache.ClassFleet, urls(fmt.Sprintf("u%d", i))) + if err == nil { + mu.Lock() + ids[id] = true + mu.Unlock() + } + }(i) + } + wg.Wait() + if len(ids) != 1 { + t.Fatalf("20 concurrent submits by one caller (cap 1) created %d jobs", len(ids)) + } + same := map[string]bool{} + for i := 0; i < 20; i++ { + wg.Add(1) + go func() { + defer wg.Done() + id, _, err := r.Submit(context.Background(), bob, dcache.ClassFleet, urls("same")) + if err == nil { + mu.Lock() + same[id] = true + mu.Unlock() + } + }() + } + wg.Wait() + if len(same) != 1 { + t.Fatalf("20 concurrent identical submits created %d jobs, want 1", len(same)) + } +} + +// A second cancel of a job with a target in flight returns false and counts once. +func TestDoubleCancel(t *testing.T) { + started := make(chan struct{}, 1) + release := make(chan struct{}) + var cancels atomic.Int64 + fetch := func(context.Context, *auth.Caller, Target) (Result, error) { + select { + case started <- struct{}{}: + default: + } + <-release + return Result{Content: "ok"}, nil + } + r := newRunner(t, fetch, Config{Hooks: Hooks{Job: func(o string) { + if o == "cancelled" { + cancels.Add(1) + } + }}}) + defer close(release) + id, _, _ := r.Submit(context.Background(), alice, dcache.ClassFleet, urls("a", "b")) + <-started + if !r.Cancel(id) { + t.Fatal("first cancel returned false") + } + if r.Cancel(id) { + t.Fatal("second cancel returned true") + } + if n := cancels.Load(); n != 1 { + t.Fatalf("cancelled counted %d times", n) + } +} + +// Negative or zero durations and sizes fall back to the defaults. +func TestConfigDefaults(t *testing.T) { + r := newRunner(t, func(context.Context, *auth.Caller, Target) (Result, error) { return Result{}, nil }, + Config{TTL: -time.Hour, MaxBytes: -1, RetryWait: -1, RetryFor: -1, TargetsPerJob: -1, JobsPerCaller: -1, JobsTotal: -1, Workers: -1}) + c := r.cfg + if c.TTL != 24*time.Hour || c.MaxBytes != 64<<20 || c.RetryWait != 2*time.Second || c.RetryFor != 10*time.Minute || + c.TargetsPerJob != 32 || c.JobsPerCaller != 1 || c.JobsTotal != 8 || c.Workers != 1 { + t.Fatalf("config %+v", c) + } +} diff --git a/internal/batch/runner_core_test.go b/internal/batch/runner_core_test.go new file mode 100644 index 0000000..6576aae --- /dev/null +++ b/internal/batch/runner_core_test.go @@ -0,0 +1,182 @@ +package batch + +// P6 task 02: the runner's core — types, Submit (idempotency, caps, storage), Status, Touch and +// Cancel — with no workers yet: every job stays queued. The helpers here are shared with task +// 03's runner_test.go. + +import ( + "context" + "errors" + "io" + "log/slog" + "testing" + "time" + + "git.wntrmute.dev/kyle/sift/internal/auth" + "git.wntrmute.dev/kyle/sift/internal/dcache" +) + +var ( + alice = &auth.Caller{Name: "alice", Workspace: "hermes-fleet"} + bob = &auth.Caller{Name: "bob", Workspace: "hermes-fleet"} + carol = &auth.Caller{Name: "carol", Workspace: "hermes-fleet"} +) + +func newRunner(t *testing.T, fetch FetchFunc, cfg Config) *Runner { + t.Helper() + db, err := dcache.Open("", 1<<20, slog.New(slog.NewTextHandler(io.Discard, nil))) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = db.Close() }) + r := New(db, fetch, cfg, slog.New(slog.NewTextHandler(io.Discard, nil))) + t.Cleanup(r.Close) // runs before db.Close (cleanups are LIFO) + return r +} + +func waitStatus(t *testing.T, r *Runner, id, want string) { + t.Helper() + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + j, _, ok := r.Status(id) + if ok && j.Status == want { + return + } + time.Sleep(2 * time.Millisecond) + } + j, _, _ := r.Status(id) + t.Fatalf("job %s never reached %q (now %q)", id, want, j.Status) +} + +func urls(ss ...string) []Target { + var out []Target + for _, s := range ss { + out = append(out, Target{URL: "https://" + s + ".example/"}) + } + return out +} + +// Submit stores the job queued with its targets pending, before it returns. +func TestSubmitStoresQueued(t *testing.T) { + r := newRunner(t, func(context.Context, *auth.Caller, Target) (Result, error) { + return Result{Content: "ok"}, nil + }, Config{Workers: 1}) + id, reused, err := r.Submit(context.Background(), alice, dcache.ClassFleet, urls("a", "b")) + if err != nil || reused || len(id) != 32 { + t.Fatalf("submit id=%q reused=%v err=%v, want a 32-hex-char id", id, reused, err) + } + j, ts, ok := r.Status(id) + if !ok || j.ID != id || j.Caller != "alice" || j.TargetsTotal != 2 || j.CreatedAt == 0 { + t.Fatalf("job %+v ok=%v", j, ok) + } + if j.Status != "queued" && j.Status != "running" && j.Status != "done" { + t.Fatalf("status %q", j.Status) + } + if len(ts) != 2 || ts[0].Idx != 0 || ts[0].URL != "https://a.example/" || ts[1].URL != "https://b.example/" { + t.Fatalf("targets %+v", ts) + } +} + +// Cancelling a job no worker has reached: it is cancelled at once, its targets stay pending, +// and the same list then starts a new job. +func TestCancelQueued(t *testing.T) { + release := make(chan struct{}) + defer close(release) + fetch := func(context.Context, *auth.Caller, Target) (Result, error) { + <-release + return Result{Content: "ok"}, nil + } + r := newRunner(t, fetch, Config{Workers: 1}) + busy, _, err := r.Submit(context.Background(), bob, dcache.ClassFleet, urls("x")) // occupies the only worker once there is one + if err != nil { + t.Fatal(err) + } + _ = busy + id, _, err := r.Submit(context.Background(), alice, dcache.ClassFleet, urls("a", "b")) + if err != nil { + t.Fatal(err) + } + if !r.Cancel(id) { + t.Fatal("cancel returned false") + } + j, ts, _ := r.Status(id) + if j.Status != "cancelled" || ts[0].Status != "pending" || ts[1].Status != "pending" { + t.Fatalf("after cancel: %+v %+v", j, ts) + } + if r.Cancel(id) || r.Cancel("no-such-job") { + t.Fatal("cancel of a finished or unknown job must return false") + } + id2, reused, err := r.Submit(context.Background(), alice, dcache.ClassFleet, urls("a", "b")) + if err != nil || reused || id2 == id { + t.Fatalf("resubmit after cancel id=%q reused=%v err=%v", id2, reused, err) + } +} + +// Touch keeps a job alive; Status never does. +func TestTouchAndStatusLease(t *testing.T) { + db, err := dcache.Open("", 1<<20, slog.New(slog.NewTextHandler(io.Discard, nil))) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = db.Close() }) + release := make(chan struct{}) + fetch := func(context.Context, *auth.Caller, Target) (Result, error) { + <-release + return Result{Content: "ok"}, nil + } + r := New(db, fetch, Config{Workers: 1}, slog.New(slog.NewTextHandler(io.Discard, nil))) + t.Cleanup(r.Close) + defer close(release) // runs before the cleanups + id, _, err := r.Submit(context.Background(), alice, dcache.ClassFleet, urls("a")) + if err != nil { + t.Fatal(err) + } + time.Sleep(50 * time.Millisecond) // let a worker (task 03) take the target; it then blocks + before, _ := db.BatchJob(id) + time.Sleep(1100 * time.Millisecond) // the lease is in whole seconds + r.Status(id) + if j, _ := db.BatchJob(id); j.Lease != before.Lease { + t.Fatalf("Status moved the lease %d → %d; a read must not write", before.Lease, j.Lease) + } + r.Touch(id) + if j, _ := db.BatchJob(id); j.Lease <= before.Lease { + t.Fatalf("Touch did not move the lease: %d → %d", before.Lease, j.Lease) + } +} + +func TestCaps(t *testing.T) { + release := make(chan struct{}) + fetch := func(context.Context, *auth.Caller, Target) (Result, error) { + <-release + return Result{Content: "ok"}, nil + } + r := newRunner(t, fetch, Config{TargetsPerJob: 2, JobsTotal: 2}) + defer close(release) + if _, _, err := r.Submit(context.Background(), alice, dcache.ClassFleet, urls("a", "b", "c")); !errors.Is(err, ErrTooManyTargets) { + t.Fatalf("3 targets over a cap of 2: want ErrTooManyTargets, got %v", err) + } + if _, _, err := r.Submit(context.Background(), alice, dcache.ClassFleet, nil); !errors.Is(err, ErrNoTargets) { + t.Fatalf("no targets: want ErrNoTargets, got %v", err) + } + if _, _, err := r.Submit(context.Background(), alice, dcache.ClassFleet, urls("a")); err != nil { + t.Fatal(err) + } + // alice's job is live (queued or running): a second, different list is refused. + if _, _, err := r.Submit(context.Background(), alice, dcache.ClassFleet, urls("z")); !errors.Is(err, ErrCallerBusy) { + t.Fatalf("want ErrCallerBusy, got %v", err) + } + if _, _, err := r.Submit(context.Background(), bob, dcache.ClassFleet, urls("b")); err != nil { + t.Fatal(err) + } + // Two live jobs in total: a third caller is refused. + if _, _, err := r.Submit(context.Background(), carol, dcache.ClassFleet, urls("c")); !errors.Is(err, ErrBusy) { + t.Fatalf("want ErrBusy, got %v", err) + } +} + +func TestStatusUnknown(t *testing.T) { + r := newRunner(t, func(context.Context, *auth.Caller, Target) (Result, error) { return Result{}, nil }, Config{}) + if _, _, ok := r.Status("no-such-job"); ok { + t.Fatal("unknown id must be ok=false") + } +} diff --git a/internal/batch/runner_test.go b/internal/batch/runner_test.go new file mode 100644 index 0000000..7d71b21 --- /dev/null +++ b/internal/batch/runner_test.go @@ -0,0 +1,373 @@ +package batch + +import ( + "context" + "errors" + "fmt" + "strings" + "sync" + "testing" + "time" + + "git.wntrmute.dev/kyle/sift/internal/auth" + "git.wntrmute.dev/kyle/sift/internal/dcache" +) + +// Targets run one at a time, in order, and each fetch is given the submitting caller. +func TestSerialOrderAndCaller(t *testing.T) { + var mu sync.Mutex + var calls, names []string + inflight, peak := 0, 0 + fetch := func(_ context.Context, c *auth.Caller, tgt Target) (Result, error) { + mu.Lock() + inflight++ + if inflight > peak { + peak = inflight + } + calls = append(calls, tgt.URL) + names = append(names, c.Name) + mu.Unlock() + time.Sleep(time.Millisecond) + mu.Lock() + inflight-- + mu.Unlock() + return Result{Content: "ok " + tgt.URL}, nil + } + r := newRunner(t, fetch, Config{}) + id, reused, err := r.Submit(context.Background(), alice, dcache.ClassFleet, urls("a", "b", "c")) + if err != nil || reused { + t.Fatalf("submit err=%v reused=%v", err, reused) + } + waitStatus(t, r, id, "done") + mu.Lock() + defer mu.Unlock() + if strings.Join(calls, " ") != "https://a.example/ https://b.example/ https://c.example/" { + t.Fatalf("order %v", calls) + } + if peak != 1 { + t.Fatalf("peak inflight %d, want 1 (serial)", peak) + } + for _, n := range names { + if n != "alice" { + t.Fatalf("fetch saw caller %q, want alice", n) + } + } + j, _, _ := r.Status(id) + if j.Caller != "alice" || j.TargetsTotal != 3 || j.TargetsDone != 3 || j.CreatedAt == 0 { + t.Fatalf("job %+v", j) + } +} + +// With one worker, two live jobs take turns target by target. +func TestRoundRobin(t *testing.T) { + first := make(chan struct{}) + release := make(chan struct{}) + var once sync.Once + var mu sync.Mutex + var calls []string + fetch := func(_ context.Context, _ *auth.Caller, tgt Target) (Result, error) { + mu.Lock() + calls = append(calls, strings.TrimSuffix(strings.TrimPrefix(tgt.URL, "https://"), ".example/")) + mu.Unlock() + once.Do(func() { close(first); <-release }) + return Result{Content: "ok"}, nil + } + r := newRunner(t, fetch, Config{Workers: 1}) + a, _, err := r.Submit(context.Background(), alice, dcache.ClassFleet, urls("a1", "a2", "a3")) + if err != nil { + t.Fatal(err) + } + <-first // a1 is in flight + b, _, err := r.Submit(context.Background(), bob, dcache.ClassFleet, urls("b1", "b2", "b3")) + if err != nil { + t.Fatal(err) + } + close(release) + waitStatus(t, r, a, "done") + waitStatus(t, r, b, "done") + mu.Lock() + defer mu.Unlock() + if got := strings.Join(calls, " "); got != "a1 b1 a2 b2 a3 b3" { + t.Fatalf("order %q, want round-robin a1 b1 a2 b2 a3 b3", got) + } +} + +func TestFailureIsolation(t *testing.T) { + fetch := func(_ context.Context, _ *auth.Caller, tgt Target) (Result, error) { + if tgt.URL == "https://b.example/" { + return Result{}, &TargetError{Msg: "upstream 500", Hint: "browser"} + } + return Result{Content: "ok " + tgt.URL}, nil + } + r := newRunner(t, fetch, Config{}) + id, _, err := r.Submit(context.Background(), alice, dcache.ClassFleet, urls("a", "b", "c")) + if err != nil { + t.Fatal(err) + } + waitStatus(t, r, id, "partial") + j, ts, ok := r.Status(id) + if !ok || j.TargetsDone != 3 { + t.Fatalf("job %+v ok=%v", j, ok) + } + if ts[0].Status != "done" || ts[1].Status != "failed" || ts[2].Status != "done" { + t.Fatalf("statuses %+v", ts) + } + if ts[1].Error != "upstream 500" || ts[1].Hint != "browser" { + t.Fatalf("failed target error=%q hint=%q", ts[1].Error, ts[1].Hint) + } + if ts[0].Result == nil || ts[0].Result.Content != "ok https://a.example/" { + t.Fatalf("result %+v", ts[0].Result) + } +} + +func TestAllFailedIsFailed(t *testing.T) { + fetch := func(context.Context, *auth.Caller, Target) (Result, error) { return Result{}, errors.New("nope") } + r := newRunner(t, fetch, Config{}) + id, _, err := r.Submit(context.Background(), alice, dcache.ClassFleet, urls("a", "b")) + if err != nil { + t.Fatal(err) + } + waitStatus(t, r, id, "failed") + _, ts, _ := r.Status(id) + if ts[0].Error != "nope" || ts[0].Hint != "" { + t.Fatalf("plain error stored as error=%q hint=%q", ts[0].Error, ts[0].Hint) + } +} + +func TestIdempotentResubmit(t *testing.T) { + var mu sync.Mutex + n := 0 + fetch := func(context.Context, *auth.Caller, Target) (Result, error) { + mu.Lock() + n++ + mu.Unlock() + return Result{Content: "ok"}, nil + } + r := newRunner(t, fetch, Config{}) + id1, reused1, err := r.Submit(context.Background(), alice, dcache.ClassFleet, urls("a", "b")) + if err != nil || reused1 { + t.Fatalf("first submit err=%v reused=%v", err, reused1) + } + id2, reused2, err := r.Submit(context.Background(), alice, dcache.ClassFleet, urls("a", "b")) + if err != nil || !reused2 || id2 != id1 { + t.Fatalf("resubmit id=%q reused=%v err=%v", id2, reused2, err) + } + waitStatus(t, r, id1, "done") + // Still reused once done (within the lease), and never re-fetched. + if id3, reused3, err := r.Submit(context.Background(), alice, dcache.ClassFleet, urls("a", "b")); err != nil || !reused3 || id3 != id1 { + t.Fatalf("resubmit after done id=%q reused=%v err=%v", id3, reused3, err) + } + // Another caller with the same list gets its own job. + if id4, reused4, err := r.Submit(context.Background(), bob, dcache.ClassFleet, urls("a", "b")); err != nil || reused4 || id4 == id1 { + t.Fatalf("other caller id=%q reused=%v err=%v", id4, reused4, err) + } + time.Sleep(50 * time.Millisecond) + mu.Lock() + defer mu.Unlock() + if n != 4 { + t.Fatalf("fetched %d times, want 4 (2 for alice's one job, 2 for bob's)", n) + } +} + +// A cancelled job is not handed back to a resubmit: the same list starts a new job. +func TestResubmitAfterCancelIsNew(t *testing.T) { + release := make(chan struct{}) + fetch := func(context.Context, *auth.Caller, Target) (Result, error) { + <-release + return Result{Content: "ok"}, nil + } + r := newRunner(t, fetch, Config{}) + id1, _, err := r.Submit(context.Background(), alice, dcache.ClassFleet, urls("a", "b")) + if err != nil { + t.Fatal(err) + } + waitStatus(t, r, id1, "running") + if !r.Cancel(id1) { + t.Fatal("cancel returned false") + } + close(release) + waitStatus(t, r, id1, "cancelled") + id2, reused, err := r.Submit(context.Background(), alice, dcache.ClassFleet, urls("a", "b")) + if err != nil || reused || id2 == id1 { + t.Fatalf("resubmit after cancel id=%q reused=%v err=%v", id2, reused, err) + } +} + +// Submit returns with the job stored "queued"; it becomes "running" when a worker takes it. +func TestQueuedThenRunning(t *testing.T) { + release := make(chan struct{}) + fetch := func(context.Context, *auth.Caller, Target) (Result, error) { + <-release + return Result{Content: "ok"}, nil + } + r := newRunner(t, fetch, Config{Workers: 1}) + defer close(release) + a, _, _ := r.Submit(context.Background(), alice, dcache.ClassFleet, urls("a")) + waitStatus(t, r, a, "running") + b, _, err := r.Submit(context.Background(), bob, dcache.ClassFleet, urls("b")) + if err != nil { + t.Fatal(err) + } + // The only worker is busy with alice's target, so bob's job waits in the queue. + time.Sleep(20 * time.Millisecond) + if j, _, _ := r.Status(b); j.Status != "queued" { + t.Fatalf("bob's job is %q while the only worker is busy, want queued", j.Status) + } +} + +func TestCancel(t *testing.T) { + started := make(chan struct{}, 1) + release := make(chan struct{}) + var mu sync.Mutex + var calls []string + fetch := func(_ context.Context, _ *auth.Caller, tgt Target) (Result, error) { + mu.Lock() + calls = append(calls, tgt.URL) + mu.Unlock() + select { + case started <- struct{}{}: + default: + } + <-release + return Result{Content: "ok"}, nil + } + r := newRunner(t, fetch, Config{}) + id, _, err := r.Submit(context.Background(), alice, dcache.ClassFleet, urls("a", "b", "c")) + if err != nil { + t.Fatal(err) + } + <-started // the first target is in flight + if !r.Cancel(id) { + t.Fatal("cancel returned false") + } + close(release) // let the in-flight target finish + waitStatus(t, r, id, "cancelled") + time.Sleep(20 * time.Millisecond) + mu.Lock() + n := len(calls) + mu.Unlock() + if n != 1 { + t.Fatalf("cancelled after %d fetches, want 1", n) + } + j, ts, ok := r.Status(id) + if !ok || j.Status != "cancelled" { + t.Fatalf("job %+v: the in-flight target's write must not overwrite cancelled", j) + } + if ts[0].Status != "done" { + t.Fatalf("in-flight target should be stored done: %+v", ts[0]) + } + if ts[1].Status != "pending" || ts[2].Status != "pending" { + t.Fatalf("later targets must stay pending: %+v", ts[1:]) + } + if r.Cancel(id) { + t.Fatal("cancelling a terminal job must return false") + } + if r.Cancel("no-such-job") { + t.Fatal("cancelling an unknown job must return false") + } +} + +// A target that reports ErrRetryLater (the fetch gate was busy) is retried, not failed. +func TestBusyIsRetried(t *testing.T) { + var mu sync.Mutex + n := 0 + fetch := func(context.Context, *auth.Caller, Target) (Result, error) { + mu.Lock() + defer mu.Unlock() + n++ + if n <= 2 { + return Result{}, fmt.Errorf("busy: %w", ErrRetryLater) + } + return Result{Content: "ok"}, nil + } + r := newRunner(t, fetch, Config{RetryWait: time.Millisecond}) + id, _, err := r.Submit(context.Background(), alice, dcache.ClassFleet, urls("a")) + if err != nil { + t.Fatal(err) + } + waitStatus(t, r, id, "done") + mu.Lock() + defer mu.Unlock() + if n != 3 { + t.Fatalf("fetched %d times, want 3 (two busy, then ok)", n) + } +} + +// Retrying is bounded: a target busy for longer than RetryFor fails with a busy error. +func TestBusyGivesUp(t *testing.T) { + fetch := func(context.Context, *auth.Caller, Target) (Result, error) { + return Result{}, fmt.Errorf("busy: %w", ErrRetryLater) + } + r := newRunner(t, fetch, Config{RetryWait: time.Millisecond, RetryFor: 30 * time.Millisecond}) + id, _, err := r.Submit(context.Background(), alice, dcache.ClassFleet, urls("a")) + if err != nil { + t.Fatal(err) + } + waitStatus(t, r, id, "failed") + _, ts, _ := r.Status(id) + if ts[0].Status != "failed" || !strings.Contains(ts[0].Error, "busy") { + t.Fatalf("target %+v, want failed with a busy error", ts[0]) + } +} + +func TestPanicRecovered(t *testing.T) { + fetch := func(_ context.Context, _ *auth.Caller, tgt Target) (Result, error) { + if tgt.URL == "https://b.example/" { + panic("boom") + } + return Result{Content: "ok"}, nil + } + r := newRunner(t, fetch, Config{}) + id, _, err := r.Submit(context.Background(), alice, dcache.ClassFleet, urls("a", "b", "c")) + if err != nil { + t.Fatal(err) + } + waitStatus(t, r, id, "partial") + _, ts, _ := r.Status(id) + if ts[0].Status != "done" || ts[1].Status != "failed" || ts[2].Status != "done" { + t.Fatalf("statuses %+v", ts) + } + if ts[1].Error == "" || strings.Contains(ts[1].Error, "boom") { + t.Fatalf("panic target error %q: must be set, and must not echo the panic value", ts[1].Error) + } +} + +// The hooks see every job outcome once and every stored target once. +func TestHooks(t *testing.T) { + var mu sync.Mutex + jobs := map[string]int{} + var targets, jobSecs int + var bytes int + h := Hooks{ + Job: func(o string) { mu.Lock(); jobs[o]++; mu.Unlock() }, + Target: func(err error, _ time.Duration, n int) { + mu.Lock() + targets++ + bytes += n + mu.Unlock() + }, + JobSeconds: func(time.Duration) { mu.Lock(); jobSecs++; mu.Unlock() }, + } + fetch := func(_ context.Context, _ *auth.Caller, tgt Target) (Result, error) { + if tgt.URL == "https://b.example/" { + return Result{}, errors.New("nope") + } + return Result{Content: "ok"}, nil + } + r := newRunner(t, fetch, Config{Hooks: h}) + id, _, _ := r.Submit(context.Background(), alice, dcache.ClassFleet, urls("a", "b")) + waitStatus(t, r, id, "partial") + _, _, _ = r.Submit(context.Background(), alice, dcache.ClassFleet, urls("a", "b")) // reused + time.Sleep(20 * time.Millisecond) + mu.Lock() + defer mu.Unlock() + if jobs["created"] != 1 || jobs["partial"] != 1 || jobs["reused"] != 1 || len(jobs) != 3 { + t.Fatalf("job outcomes %v", jobs) + } + if targets != 2 || jobSecs != 1 { + t.Fatalf("targets=%d jobSeconds=%d, want 2 and 1", targets, jobSecs) + } + if bytes <= 0 { + t.Fatalf("stored result bytes %d, want > 0 for the done target", bytes) + } +} diff --git a/internal/batch/worker.go b/internal/batch/worker.go new file mode 100644 index 0000000..caf223a --- /dev/null +++ b/internal/batch/worker.go @@ -0,0 +1,286 @@ +package batch + +import ( + "context" + "encoding/json" + "errors" + "time" + + "git.wntrmute.dev/kyle/sift/internal/auth" +) + +// ErrInternal is stored for a target whose fetch panicked; the panic value is never echoed. +var ErrInternal = errors.New("internal error") + +// liveJob is a submitted job that the workers are draining. It lives in Runner.live until the +// last target is stored or the job is cancelled. +type liveJob struct { + id string + caller *auth.Caller + targets []Target // as submitted + created time.Time + next int // index of the next target to fetch + running bool // a target of this job is in flight + started bool // set to "running" in the store already + cancelled bool + retryAt time.Time // zero, or when the busy target may be tried again + busySince time.Time // zero, or when the current target first came back busy + done, failed int +} + +// worker drains the rotation: it takes the next live job with a pending target and no target in +// flight, fetches and stores that one target, and moves on. Each job is serial — its target i+1 +// never starts before i is stored. +func (r *Runner) worker() { + defer r.wg.Done() + + for { + r.mu.Lock() + if r.closed { // stop taking targets at once; live jobs are reaped on the next start + r.mu.Unlock() + return + } + now := time.Now() + j := r.pick(now) + if j == nil { + r.scheduleRetryLocked(now) + r.cond.Wait() // re-locks on wake; release before the next top-of-loop Lock + r.mu.Unlock() + continue + } + + // Take the target: mark the job running and bump the in-flight count under the lock, + // then fetch outside it. + j.running = true + r.inflight++ + r.callInflight(r.inflight) + if !j.started { + if err := r.db.SetBatchJobStatus(j.id, "running", "", time.Now().Unix()); err != nil { + r.log.Warn("batch: set running", "job", j.id, "err", err) + } + j.started = true + } + idx := j.next + target := j.targets[idx] + startedAt := time.Now() + r.mu.Unlock() + + res, err := r.runTarget(r.ctx, j, idx, target) + + r.mu.Lock() + now = time.Now() // after the fetch: a busy answer can take seconds to come back + j.running = false + r.inflight-- + r.callInflight(r.inflight) + + // Shutting down: a failure now is most likely the cancelled context. Store nothing; the + // job stays live in the store for the restart reaper. + if r.closed && err != nil { + r.cond.Broadcast() + r.mu.Unlock() + continue + } + + // A cancelled job whose in-flight target was only busy is finished: nothing more runs. + if errors.Is(err, ErrRetryLater) && j.cancelled { + r.removeLocked(j) + r.callDepth(len(r.live)) + r.cond.Broadcast() + r.mu.Unlock() + continue + } + + // A target that was only busy stays pending and is retried after RetryWait, unless it + // has been busy for longer than RetryFor, in which case it is stored failed below. + if errors.Is(err, ErrRetryLater) { + if j.busySince.IsZero() { + j.busySince = now + } + if now.Sub(j.busySince) <= r.cfg.RetryFor { + j.retryAt = now.Add(r.cfg.RetryWait) + r.cond.Broadcast() + r.mu.Unlock() + continue + } + } + + resultBytes := r.storeTarget(j, idx, res, err) + j.next++ + j.busySince, j.retryAt = time.Time{}, time.Time{} + if err == nil { + j.done++ + } else { + j.failed++ + } + r.callTarget(err, time.Since(startedAt), resultBytes) + r.db.EvictBatchOldest(r.cfg.MaxBytes) + + if j.cancelled || j.next >= len(j.targets) { + if !j.cancelled { + r.setTerminal(j) + } + r.removeLocked(j) + r.callDepth(len(r.live)) + } + r.cond.Broadcast() + r.mu.Unlock() + } +} + +// pick returns the first live job, scanning from cursor and wrapping once, that has a pending +// target that is not running, not cancelled, and ready (now past any retry). It advances cursor to +// the index after the job, so the rotation is round-robin across jobs in submit order. +func (r *Runner) pick(now time.Time) *liveJob { + n := len(r.live) + if n == 0 { + return nil + } + for i := 0; i < n; i++ { + j := r.live[(r.cursor+i)%n] + if j.running || j.cancelled || j.next >= len(j.targets) { + continue + } + if !now.Before(j.retryAt) { // now >= retryAt; a zero retryAt is always ready + // The job after this one; n (one past the end) means "whatever is appended next, + // else wrap to 0" — reads take cursor % n. + r.cursor = (r.cursor+i)%n + 1 + return j + } + } + return nil +} + +// scheduleRetryLocked arms a timer that wakes the workers at the earliest future retryAt, so they +// do not sleep past a busy target that is ready to be tried again. +func (r *Runner) scheduleRetryLocked(now time.Time) { + var earliest time.Time + for _, j := range r.live { + if j.cancelled || j.next >= len(j.targets) || !j.retryAt.After(now) { + continue + } + if earliest.IsZero() || j.retryAt.Before(earliest) { + earliest = j.retryAt + } + } + if !earliest.IsZero() { + d := time.Until(earliest) + if d < 0 { + d = 0 + } + time.AfterFunc(d, r.cond.Broadcast) + } +} + +// removeLocked drops j from live and fixes cursor so the rotation still points at the next job. +func (r *Runner) removeLocked(j *liveJob) { + idx := -1 + for i := range r.live { + if r.live[i] == j { + idx = i + break + } + } + if idx < 0 { + return + } + r.live = append(r.live[:idx], r.live[idx+1:]...) + if r.cursor > idx { + r.cursor-- + } + if r.cursor > len(r.live) { + r.cursor = len(r.live) + } +} + +// setTerminal writes the job's finished status once: all done → done, all failed → failed, a mix +// → partial. +func (r *Runner) setTerminal(j *liveJob) { + var status string + switch { + case j.failed == 0: + status = "done" + case j.done == 0: + status = "failed" + default: + status = "partial" + } + if err := r.db.SetBatchJobStatus(j.id, status, "", time.Now().Unix()); err != nil { + r.log.Warn("batch: set terminal status", "job", j.id, "err", err) + } + r.callJob(status) + r.callJobSeconds(time.Since(j.created)) +} + +// runTarget fetches one target, recovering a panic into ErrInternal (never echoing the value). +func (r *Runner) runTarget(ctx context.Context, j *liveJob, idx int, t Target) (res Result, err error) { + defer func() { + if rec := recover(); rec != nil { + r.log.Warn("batch: recover fetch panic", "job", j.id, "idx", idx) + res = Result{} + err = ErrInternal + } + }() + return r.fetch(ctx, j.caller, t) +} + +// storeTarget stores one target: success → done with the result JSON, error → failed with the +// TargetError's Msg/Hint or the error's text. It returns the number of result bytes stored. +func (r *Runner) storeTarget(j *liveJob, idx int, res Result, err error) int { + now := time.Now().Unix() + status := "failed" + var resultStr, errMsg, hint *string + if err == nil { + status = "done" + b, merr := json.Marshal(res) + if merr != nil { + r.log.Warn("batch: marshal result", "job", j.id, "idx", idx, "err", merr) + } else { + s := string(b) + resultStr = &s + } + } else { + var terr *TargetError + if errors.As(err, &terr) { + m := terr.Msg + errMsg = &m + if terr.Hint != "" { + h := terr.Hint + hint = &h + } + } else { + e := err.Error() + errMsg = &e + } + } + if serr := r.db.UpdateBatchTarget(j.id, idx, now, status, resultStr, errMsg, hint); serr != nil { + r.log.Warn("batch: store target", "job", j.id, "idx", idx, "err", serr) + } + if resultStr != nil { + return len(*resultStr) + } + return 0 +} + +func (r *Runner) callDepth(live int) { + if r.cfg.Hooks.Depth != nil { + r.cfg.Hooks.Depth(live) + } +} + +func (r *Runner) callInflight(n int) { + if r.cfg.Hooks.Inflight != nil { + r.cfg.Hooks.Inflight(n) + } +} + +func (r *Runner) callTarget(err error, d time.Duration, resultBytes int) { + if r.cfg.Hooks.Target != nil { + r.cfg.Hooks.Target(err, d, resultBytes) + } +} + +func (r *Runner) callJobSeconds(d time.Duration) { + if r.cfg.Hooks.JobSeconds != nil { + r.cfg.Hooks.JobSeconds(d) + } +} diff --git a/internal/config/batch_review_test.go b/internal/config/batch_review_test.go new file mode 100644 index 0000000..56ff0bb --- /dev/null +++ b/internal/config/batch_review_test.go @@ -0,0 +1,10 @@ +package config + +import "testing" + +// P6 owner review: a size too large for int64 falls back to the default instead of wrapping negative. +func TestParseSizeOverflow(t *testing.T) { + if got := ParseSize("9223372036854775807GiB", 7); got != 7 { + t.Fatalf("ParseSize(huge) = %d, want the default", got) + } +} diff --git a/internal/config/batch_test.go b/internal/config/batch_test.go new file mode 100644 index 0000000..b4ea9c1 --- /dev/null +++ b/internal/config/batch_test.go @@ -0,0 +1,56 @@ +package config + +// P6 task 08: the [batch] block and ParseSize. + +import ( + "os" + "path/filepath" + "testing" +) + +func TestBatchBlock(t *testing.T) { + p := filepath.Join(t.TempDir(), "sift.toml") + body := "[batch]\ndisabled = false\ntargets_per_job = 16\njobs_per_caller = 2\njobs_total = 4\nworkers = 2\nttl = \"12h\"\nmax_bytes = \"32MiB\"\n" + if err := os.WriteFile(p, []byte(body), 0o600); err != nil { + t.Fatal(err) + } + f, err := Load(p) + if err != nil { + t.Fatal(err) + } + b := f.Batch + if b.Disabled || b.TargetsPerJob != 16 || b.JobsPerCaller != 2 || b.JobsTotal != 4 || b.Workers != 2 || b.TTL != "12h" || b.MaxBytes != "32MiB" { + t.Fatalf("batch block %+v", b) + } + // No block: zero values, and batch is on (the runner applies its own defaults). + empty := filepath.Join(t.TempDir(), "empty.toml") + if err := os.WriteFile(empty, []byte("listen = \":8443\"\n"), 0o600); err != nil { + t.Fatal(err) + } + if f, err := Load(empty); err != nil || f.Batch != (Batch{}) { + t.Fatalf("no [batch] block: %+v %v", f.Batch, err) + } +} + +func TestParseSize(t *testing.T) { + const def = 7 + for _, c := range []struct { + in string + want int64 + }{ + {"64MiB", 64 << 20}, + {"512KiB", 512 << 10}, + {"1GiB", 1 << 30}, + {"1000", 1000}, + {" 2MiB ", 2 << 20}, + {"", def}, + {"junk", def}, + {"-1MiB", def}, + {"0", def}, + {"12MB", def}, // decimal units are not accepted: say MiB + } { + if got := ParseSize(c.in, def); got != c.want { + t.Errorf("ParseSize(%q) = %d, want %d", c.in, got, c.want) + } + } +} diff --git a/internal/config/config.go b/internal/config/config.go index e0205de..4dc5299 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1,7 +1,10 @@ package config import ( + "math" "os" + "strconv" + "strings" "time" "github.com/pelletier/go-toml/v2" @@ -17,6 +20,19 @@ type File struct { Digest Digest `toml:"digest"` Context Context `toml:"context"` Search Search `toml:"search"` + Batch Batch `toml:"batch"` +} + +// Batch is the [batch] block. Batch is on by default: no block means the runner runs with its own +// defaults. disabled = true turns it off and the endpoints answer 503. +type Batch struct { + Disabled bool `toml:"disabled"` + TargetsPerJob int `toml:"targets_per_job"` + JobsPerCaller int `toml:"jobs_per_caller"` + JobsTotal int `toml:"jobs_total"` + Workers int `toml:"workers"` + TTL string `toml:"ttl"` + MaxBytes string `toml:"max_bytes"` } type Dixie struct { @@ -110,3 +126,28 @@ func ParseDuration(s string, def time.Duration) time.Duration { } return d } + +// ParseSize parses a positive byte count: a plain integer, or one with a binary suffix KiB, MiB or +// GiB. Anything else, zero or negative returns def. +func ParseSize(s string, def int64) int64 { + s = strings.TrimSpace(s) + if s == "" { + return def + } + var mult int64 + switch { + case strings.HasSuffix(s, "KiB"): + mult, s = 1<<10, s[:len(s)-3] + case strings.HasSuffix(s, "MiB"): + mult, s = 1<<20, s[:len(s)-3] + case strings.HasSuffix(s, "GiB"): + mult, s = 1<<30, s[:len(s)-3] + default: + mult = 1 + } + n, err := strconv.ParseInt(s, 10, 64) + if err != nil || n <= 0 || n > math.MaxInt64/mult { // the last: n*mult would overflow + return def + } + return n * mult +} diff --git a/internal/dcache/batch.go b/internal/dcache/batch.go new file mode 100644 index 0000000..8a44759 --- /dev/null +++ b/internal/dcache/batch.go @@ -0,0 +1,342 @@ +package dcache + +import ( + "database/sql" + "fmt" + "time" +) + +// terminal statuses mark a target as finished; a target already in one is not +// counted again on a re-write. +func isTerminal(status string) bool { + return status == "done" || status == "failed" +} + +type BatchJob struct { + ID string + Caller string + CallerClass string + IDemKey string + Status string + Error string + TargetsTotal int + TargetsDone int + CreatedAt int64 + Lease int64 + NBytes int64 +} + +type BatchTarget struct { + JobID string + Idx int + URL string + Options string + Status string + Result *string + Error *string + Hint *string +} + +func nullString(s *string) sql.NullString { + if s == nil { + return sql.NullString{Valid: false} + } + return sql.NullString{String: *s, Valid: true} +} + +func nullPtr(n sql.NullString) *string { + if !n.Valid { + return nil + } + s := n.String + return &s +} + +func (d *DB) CreateBatchJob(j BatchJob) error { + if d == nil { + return fmt.Errorf("dcache: nil db") + } + _, err := d.sql.Exec(` + INSERT INTO batch_job + (id, caller, caller_class, idem_key, status, error, targets_total, targets_done, created_at, lease, nbytes) + VALUES(?,?,?,?,?,?,?,?,?,?,?) + `, j.ID, j.Caller, j.CallerClass, j.IDemKey, j.Status, j.Error, j.TargetsTotal, j.TargetsDone, j.CreatedAt, j.Lease, j.NBytes) + if err != nil { + return err + } + return nil +} + +func (d *DB) BatchJob(id string) (BatchJob, bool) { + if d == nil { + return BatchJob{}, false + } + var j BatchJob + err := d.sql.QueryRow(` + SELECT id, caller, caller_class, idem_key, status, error, + targets_total, targets_done, created_at, lease, nbytes + FROM batch_job WHERE id = ? + `, id).Scan(&j.ID, &j.Caller, &j.CallerClass, &j.IDemKey, &j.Status, &j.Error, + &j.TargetsTotal, &j.TargetsDone, &j.CreatedAt, &j.Lease, &j.NBytes) + if err != nil { + d.mu.Lock() + d.BatchM++ + d.mu.Unlock() + return BatchJob{}, false + } + d.mu.Lock() + d.BatchH++ + d.mu.Unlock() + return j, true +} + +// BatchJobByIDem returns the id of the newest job for (caller, idemKey) that is +// still within its lease and not failed/cancelled. Reads never write. +func (d *DB) BatchJobByIDem(caller, idemKey string, now int64, ttl time.Duration) (string, bool) { + if d == nil { + return "", false + } + ttlSec := int64(ttl.Seconds()) + var id string + err := d.sql.QueryRow(` + SELECT id FROM batch_job + WHERE caller = ? AND idem_key = ? AND lease + ? > ? + AND status IN ('queued','running','done','partial') + ORDER BY created_at DESC LIMIT 1 + `, caller, idemKey, ttlSec, now).Scan(&id) + if err != nil { + return "", false + } + return id, true +} + +func (d *DB) LiveBatchJobs(caller string) int { + if d == nil { + return 0 + } + var n int + _ = d.sql.QueryRow(` + SELECT COUNT(*) FROM batch_job WHERE caller = ? AND status IN ('queued','running') + `, caller).Scan(&n) + return n +} + +func (d *DB) LiveBatchJobsTotal() int { + if d == nil { + return 0 + } + var n int + _ = d.sql.QueryRow(`SELECT COUNT(*) FROM batch_job WHERE status IN ('queued','running')`).Scan(&n) + return n +} + +func (d *DB) PutBatchTargets(jobID string, targets []BatchTarget) error { + if d == nil { + return fmt.Errorf("dcache: nil db") + } + // One transaction: a job never has only some of its targets. + tx, err := d.sql.Begin() + if err != nil { + return err + } + defer func() { _ = tx.Rollback() }() + for _, t := range targets { + if _, err := tx.Exec(` + INSERT INTO batch_target(job_id, idx, url, options, status, result, error, hint) + VALUES(?,?,?,?,?,?,?,?) + `, t.JobID, t.Idx, t.URL, t.Options, t.Status, + nullString(t.Result), nullString(t.Error), nullString(t.Hint)); err != nil { + return err + } + } + return tx.Commit() +} + +func (d *DB) BatchTargets(jobID string) ([]BatchTarget, bool) { + if d == nil { + return nil, false + } + rows, err := d.sql.Query(` + SELECT job_id, idx, url, options, status, result, error, hint + FROM batch_target WHERE job_id = ? ORDER BY idx ASC + `, jobID) + if err != nil { + d.mu.Lock() + d.BatchM++ + d.mu.Unlock() + return nil, false + } + defer rows.Close() + var out []BatchTarget + for rows.Next() { + var ( + r, e, h sql.NullString + t BatchTarget + ) + if err := rows.Scan(&t.JobID, &t.Idx, &t.URL, &t.Options, &t.Status, &r, &e, &h); err != nil { + d.mu.Lock() + d.BatchM++ + d.mu.Unlock() + return nil, false + } + t.Result = nullPtr(r) + t.Error = nullPtr(e) + t.Hint = nullPtr(h) + out = append(out, t) + } + if err := rows.Err(); err != nil { + d.mu.Lock() + d.BatchM++ + d.mu.Unlock() + return nil, false + } + if len(out) == 0 { + return nil, false + } + return out, true +} + +func (d *DB) UpdateBatchTarget(jobID string, idx int, now int64, status string, result, errMsg, hint *string) error { + if d == nil { + return fmt.Errorf("dcache: nil db") + } + tx, err := d.sql.Begin() + if err != nil { + return err + } + defer tx.Rollback() + + var prev string + err = tx.QueryRow(`SELECT status FROM batch_target WHERE job_id = ? AND idx = ?`, jobID, idx).Scan(&prev) + if err != nil { + return err + } + + var resBytes int64 + if result != nil { + resBytes = int64(len(*result)) + } + deltaDone := 0 + if isTerminal(status) && !isTerminal(prev) { + deltaDone = 1 + } + + if _, err := tx.Exec(` + UPDATE batch_target SET status = ?, result = ?, error = ?, hint = ? WHERE job_id = ? AND idx = ? + `, status, nullString(result), nullString(errMsg), nullString(hint), jobID, idx); err != nil { + return err + } + if _, err := tx.Exec(` + UPDATE batch_job SET lease = ?, targets_done = targets_done + ?, nbytes = nbytes + ? WHERE id = ? + `, now, deltaDone, resBytes, jobID); err != nil { + return err + } + return tx.Commit() +} + +func (d *DB) SetBatchJobStatus(id, status, errMsg string, now int64) error { + if d == nil { + return fmt.Errorf("dcache: nil db") + } + _, err := d.sql.Exec(`UPDATE batch_job SET status = ?, error = ?, lease = ? WHERE id = ?`, status, errMsg, now, id) + return err +} + +func (d *DB) TouchBatchJob(id string, now int64) error { + if d == nil { + return fmt.Errorf("dcache: nil db") + } + _, err := d.sql.Exec(`UPDATE batch_job SET lease = ? WHERE id = ?`, now, id) + return err +} + +func (d *DB) SweepBatch(now int64, ttl time.Duration) int { + if d == nil { + return 0 + } + ttlSec := int64(ttl.Seconds()) + // A queued or running job is never swept: it is still in a worker's rotation. The restart + // reaper marks such jobs failed, and they are swept after that. Both deletes are one + // transaction, so a job never loses its targets but keeps its row. + tx, err := d.sql.Begin() + if err != nil { + return 0 + } + defer func() { _ = tx.Rollback() }() + const expired = `SELECT id FROM batch_job WHERE lease + ? <= ? AND status NOT IN ('queued','running')` + if _, err := tx.Exec(`DELETE FROM batch_target WHERE job_id IN (`+expired+`)`, ttlSec, now); err != nil { + return 0 + } + res, err := tx.Exec(`DELETE FROM batch_job WHERE id IN (`+expired+`)`, ttlSec, now) + if err != nil { + return 0 + } + if err := tx.Commit(); err != nil { + return 0 + } + n, _ := res.RowsAffected() + return int(n) +} + +func (d *DB) EvictBatchOldest(maxBytes int64) int { + if d == nil { + return 0 + } + evicted := 0 + for d.BatchNbytes() > maxBytes { + var id string + err := d.sql.QueryRow(` + SELECT id FROM batch_job + WHERE status NOT IN ('queued','running') + ORDER BY lease ASC, id ASC + LIMIT 1 + `).Scan(&id) + if err != nil { + break + } + if err := d.deleteBatchJob(id); err != nil { + break + } + evicted++ + } + return evicted +} + +func (d *DB) MarkInFlightFailed(reason string, now int64) int { + if d == nil { + return 0 + } + res, err := d.sql.Exec(` + UPDATE batch_job SET status = 'failed', error = ?, lease = ? WHERE status IN ('queued','running') + `, reason, now) + if err != nil { + return 0 + } + n, _ := res.RowsAffected() + return int(n) +} + +func (d *DB) BatchNbytes() int64 { + if d == nil { + return 0 + } + var n sql.NullInt64 + _ = d.sql.QueryRow(`SELECT IFNULL(SUM(nbytes),0) FROM batch_job`).Scan(&n) + return n.Int64 +} + +// deleteBatchJob removes a job and its targets in one transaction. +func (d *DB) deleteBatchJob(id string) error { + tx, err := d.sql.Begin() + if err != nil { + return err + } + defer func() { _ = tx.Rollback() }() + if _, err := tx.Exec(`DELETE FROM batch_target WHERE job_id = ?`, id); err != nil { + return err + } + if _, err := tx.Exec(`DELETE FROM batch_job WHERE id = ?`, id); err != nil { + return err + } + return tx.Commit() +} diff --git a/internal/dcache/batch_review_test.go b/internal/dcache/batch_review_test.go new file mode 100644 index 0000000..841becf --- /dev/null +++ b/internal/dcache/batch_review_test.go @@ -0,0 +1,25 @@ +package dcache + +// P6 owner review: the sweep never removes a live job. + +import ( + "testing" + "time" +) + +func TestSweepSparesLiveJobs(t *testing.T) { + db := newBatchDB(t) + for _, st := range []string{"queued", "running", "done"} { + if err := db.CreateBatchJob(BatchJob{ID: st, Caller: "a", CallerClass: ClassFleet, IDemKey: st, Status: st, TargetsTotal: 1, CreatedAt: 1, Lease: 1}); err != nil { + t.Fatal(err) + } + } + if n := db.SweepBatch(1_000_000, time.Hour); n != 1 { + t.Fatalf("swept %d, want only the done job", n) + } + for _, id := range []string{"queued", "running"} { + if _, ok := db.BatchJob(id); !ok { + t.Fatalf("live job %s was swept", id) + } + } +} diff --git a/internal/dcache/batch_test.go b/internal/dcache/batch_test.go new file mode 100644 index 0000000..c9afc19 --- /dev/null +++ b/internal/dcache/batch_test.go @@ -0,0 +1,254 @@ +package dcache + +import ( + "io" + "log/slog" + "strings" + "testing" + "time" +) + +func newBatchDB(t *testing.T) *DB { + t.Helper() + db, err := Open("", 1<<20, slog.New(slog.NewTextHandler(io.Discard, nil))) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = db.Close() }) + return db +} + +func strp(s string) *string { return &s } + +func TestBatchJobRoundTrip(t *testing.T) { + db := newBatchDB(t) + j := BatchJob{ + ID: "j1", Caller: "alice", CallerClass: ClassFleet, IDemKey: "k1", + Status: "queued", TargetsTotal: 3, CreatedAt: 100, Lease: 100, + } + if err := db.CreateBatchJob(j); err != nil { + t.Fatal(err) + } + got, ok := db.BatchJob("j1") + if !ok { + t.Fatal("missing job") + } + if got.ID != j.ID || got.Caller != "alice" || got.CallerClass != ClassFleet || + got.Status != "queued" || got.TargetsTotal != 3 || got.Lease != 100 || got.CreatedAt != 100 { + t.Fatalf("round trip mismatch: %+v", got) + } + if _, ok := db.BatchJob("nope"); ok { + t.Fatal("unknown id must be ok=false") + } +} + +func TestBatchIDemLookup(t *testing.T) { + db := newBatchDB(t) + if err := db.CreateBatchJob(BatchJob{ID: "j1", Caller: "a", CallerClass: ClassFleet, IDemKey: "k", Status: "running", TargetsTotal: 1, CreatedAt: 100, Lease: 1000}); err != nil { + t.Fatal(err) + } + if id, ok := db.BatchJobByIDem("a", "k", 2000, time.Hour); !ok || id != "j1" { + t.Fatalf("live idem: %q %v", id, ok) + } + if _, ok := db.BatchJobByIDem("a", "k", 5000, time.Hour); ok { + t.Fatal("lease-expired job must not match") + } + if _, ok := db.BatchJobByIDem("b", "k", 2000, time.Hour); ok { + t.Fatal("other caller must not match") + } + // Only a job that is live or has results is reused. A failed (e.g. killed by a restart) + // or cancelled job must not be handed back to a resubmit for the rest of its lease. + for _, c := range []struct { + status string + reuse bool + }{{"queued", true}, {"done", true}, {"partial", true}, {"failed", false}, {"cancelled", false}} { + key := "k-" + c.status + if err := db.CreateBatchJob(BatchJob{ID: "j-" + c.status, Caller: "a", CallerClass: ClassFleet, IDemKey: key, Status: c.status, TargetsTotal: 1, CreatedAt: 100, Lease: 1000}); err != nil { + t.Fatal(err) + } + if _, ok := db.BatchJobByIDem("a", key, 2000, time.Hour); ok != c.reuse { + t.Errorf("status %s: reused=%v, want %v", c.status, ok, c.reuse) + } + } +} + +func TestBatchLiveCounts(t *testing.T) { + db := newBatchDB(t) + mk := func(id, caller, status string) { + if err := db.CreateBatchJob(BatchJob{ID: id, Caller: caller, CallerClass: ClassFleet, IDemKey: id, Status: status, TargetsTotal: 1, CreatedAt: 1, Lease: 1}); err != nil { + t.Fatal(err) + } + } + mk("j1", "a", "queued") + mk("j2", "a", "running") + mk("j3", "a", "done") + mk("j4", "b", "running") + if n := db.LiveBatchJobs("a"); n != 2 { + t.Fatalf("caller a live=%d want 2", n) + } + if n := db.LiveBatchJobsTotal(); n != 3 { + t.Fatalf("total live=%d want 3", n) + } +} + +func TestBatchTargetsOrderedAndUpdate(t *testing.T) { + db := newBatchDB(t) + must := func(err error) { + t.Helper() + if err != nil { + t.Fatal(err) + } + } + must(db.CreateBatchJob(BatchJob{ID: "j1", Caller: "a", CallerClass: ClassFleet, IDemKey: "k", Status: "running", TargetsTotal: 3, CreatedAt: 100, Lease: 100})) + must(db.PutBatchTargets("j1", []BatchTarget{ + {JobID: "j1", Idx: 0, URL: "https://a.example/", Options: `{"mode":"clean"}`, Status: "pending"}, + {JobID: "j1", Idx: 1, URL: "https://b.example/", Status: "pending"}, + {JobID: "j1", Idx: 2, URL: "https://c.example/", Status: "pending"}, + })) + got, ok := db.BatchTargets("j1") + if !ok || len(got) != 3 { + t.Fatalf("targets len=%d ok=%v", len(got), ok) + } + for i := range got { + if got[i].Idx != i { + t.Fatalf("order broken: got[%d].Idx=%d", i, got[i].Idx) + } + } + + // A done write bumps the lease to now and counts the target once. + res := strp(`{"content":"ok"}`) + must(db.UpdateBatchTarget("j1", 0, 200, "done", res, nil, nil)) + j, _ := db.BatchJob("j1") + if j.Lease != 200 || j.TargetsDone != 1 { + t.Fatalf("after first write lease=%d done=%d", j.Lease, j.TargetsDone) + } + // Re-writing the same target (still a done transition) must not double-count. + must(db.UpdateBatchTarget("j1", 0, 201, "done", res, nil, nil)) + j, _ = db.BatchJob("j1") + if j.TargetsDone != 1 { + t.Fatalf("double-counted: done=%d", j.TargetsDone) + } + // A failed write counts too. + must(db.UpdateBatchTarget("j1", 1, 202, "failed", nil, strp("upstream 500"), strp("browser"))) + j, _ = db.BatchJob("j1") + if j.TargetsDone != 2 || j.Lease != 202 { + t.Fatalf("after failed write lease=%d done=%d", j.Lease, j.TargetsDone) + } +} + +func TestBatchTouchAndStatus(t *testing.T) { + db := newBatchDB(t) + if err := db.CreateBatchJob(BatchJob{ID: "j1", Caller: "a", CallerClass: ClassFleet, IDemKey: "k", Status: "running", TargetsTotal: 1, CreatedAt: 100, Lease: 100}); err != nil { + t.Fatal(err) + } + if err := db.TouchBatchJob("j1", 500); err != nil { + t.Fatal(err) + } + if j, _ := db.BatchJob("j1"); j.Lease != 500 { + t.Fatalf("touch lease=%d want 500", j.Lease) + } + if err := db.SetBatchJobStatus("j1", "cancelled", "", 600); err != nil { + t.Fatal(err) + } + if j, _ := db.BatchJob("j1"); j.Status != "cancelled" || j.Lease != 600 { + t.Fatalf("status=%q lease=%d", j.Status, j.Lease) + } +} + +func TestSweepBatch(t *testing.T) { + db := newBatchDB(t) + mk := func(id string, lease int64) { + if err := db.CreateBatchJob(BatchJob{ID: id, Caller: "a", CallerClass: ClassFleet, IDemKey: id, Status: "done", TargetsTotal: 1, CreatedAt: 1, Lease: lease}); err != nil { + t.Fatal(err) + } + if err := db.PutBatchTargets(id, []BatchTarget{{JobID: id, Idx: 0, URL: "https://x.example/", Status: "done", Result: strp("x")}}); err != nil { + t.Fatal(err) + } + } + mk("old", 100) + mk("new", 1000) + n := db.SweepBatch(300, 100*time.Second) // old: 100+100=200<=300 expired; new: 1100>300 kept + if n != 1 { + t.Fatalf("swept %d want 1", n) + } + if _, ok := db.BatchJob("old"); ok { + t.Fatal("old job must be gone") + } + if _, ok := db.BatchJob("new"); !ok { + t.Fatal("new job must remain") + } + if _, ok := db.BatchTargets("old"); ok { + t.Fatal("old targets must be gone") + } +} + +func TestEvictBatchOldest(t *testing.T) { + db := newBatchDB(t) + done := func(id string, lease int64, size int) { + if err := db.CreateBatchJob(BatchJob{ID: id, Caller: "a", CallerClass: ClassFleet, IDemKey: id, Status: "done", TargetsTotal: 1, CreatedAt: 1, Lease: lease}); err != nil { + t.Fatal(err) + } + if err := db.PutBatchTargets(id, []BatchTarget{{JobID: id, Idx: 0, URL: "https://x.example/", Status: "pending"}}); err != nil { + t.Fatal(err) + } + if err := db.UpdateBatchTarget(id, 0, lease, "done", strp(strings.Repeat("x", size)), nil, nil); err != nil { + t.Fatal(err) + } + } + done("old1", 100, 100) + done("old2", 200, 200) + done("new", 300, 300) + + // A running job with one completed target must never be byte-evicted. + if err := db.CreateBatchJob(BatchJob{ID: "run", Caller: "a", CallerClass: ClassFleet, IDemKey: "run", Status: "running", TargetsTotal: 2, TargetsDone: 1, CreatedAt: 1, Lease: 400}); err != nil { + t.Fatal(err) + } + if err := db.PutBatchTargets("run", []BatchTarget{ + {JobID: "run", Idx: 0, URL: "https://x.example/", Status: "pending"}, + {JobID: "run", Idx: 1, URL: "https://y.example/", Status: "pending"}, + }); err != nil { + t.Fatal(err) + } + if err := db.UpdateBatchTarget("run", 0, 400, "done", strp(strings.Repeat("x", 500)), nil, nil); err != nil { + t.Fatal(err) + } + + // total nbytes = 100+200+300+500 = 1100. Evict oldest done until <= 250: + // old1(100), old2(200), new(300) all evicted (oldest first); run stays (live). + db.EvictBatchOldest(250) + for _, id := range []string{"old1", "old2", "new"} { + if _, ok := db.BatchJob(id); ok { + t.Fatalf("%s should be byte-evicted", id) + } + } + if _, ok := db.BatchJob("run"); !ok { + t.Fatal("running job must not be byte-evicted") + } + if n := db.BatchNbytes(); n != 500 { + t.Fatalf("BatchNbytes=%d want 500", n) + } +} + +func TestMarkInFlightFailed(t *testing.T) { + db := newBatchDB(t) + mk := func(id, status string) { + if err := db.CreateBatchJob(BatchJob{ID: id, Caller: "a", CallerClass: ClassFleet, IDemKey: id, Status: status, TargetsTotal: 1, CreatedAt: 1, Lease: 1}); err != nil { + t.Fatal(err) + } + } + mk("q", "queued") + mk("r", "running") + mk("d", "done") + if n := db.MarkInFlightFailed("restart", 999); n != 2 { + t.Fatalf("marked %d want 2", n) + } + if j, _ := db.BatchJob("q"); j.Status != "failed" || j.Error != "restart" || j.Lease != 999 { + t.Fatalf("q %+v", j) + } + if j, _ := db.BatchJob("r"); j.Status != "failed" || j.Error != "restart" { + t.Fatalf("r %+v", j) + } + if j, _ := db.BatchJob("d"); j.Status != "done" { + t.Fatalf("done job changed: %+v", j) + } +} diff --git a/internal/dcache/dcache.go b/internal/dcache/dcache.go index ebff459..579cb13 100644 --- a/internal/dcache/dcache.go +++ b/internal/dcache/dcache.go @@ -47,6 +47,8 @@ type DB struct { SearchM uint64 PDFH uint64 PDFM uint64 + BatchH uint64 + BatchM uint64 Evictions uint64 } @@ -110,6 +112,32 @@ func Open(path string, maxBytes int64, log *slog.Logger) (*DB, error) { text TEXT, PRIMARY KEY (k, page) ); + CREATE TABLE IF NOT EXISTS batch_job ( + id TEXT PRIMARY KEY, + caller TEXT NOT NULL, + caller_class TEXT NOT NULL, + idem_key TEXT NOT NULL, + status TEXT NOT NULL, + error TEXT NOT NULL DEFAULT '', + targets_total INTEGER NOT NULL, + targets_done INTEGER NOT NULL DEFAULT 0, + created_at INTEGER NOT NULL, + lease INTEGER NOT NULL, + nbytes INTEGER NOT NULL DEFAULT 0 + ); + CREATE TABLE IF NOT EXISTS batch_target ( + job_id TEXT NOT NULL, + idx INTEGER NOT NULL, + url TEXT NOT NULL, + options TEXT NOT NULL, + status TEXT NOT NULL, + result TEXT, + error TEXT, + hint TEXT, + PRIMARY KEY (job_id, idx) + ); + CREATE INDEX IF NOT EXISTS batch_job_idem ON batch_job(caller, idem_key); + CREATE INDEX IF NOT EXISTS batch_job_lease ON batch_job(lease); `); err != nil { _ = sq.Close() return nil, err diff --git a/internal/metrics/batch.go b/internal/metrics/batch.go new file mode 100644 index 0000000..f830456 --- /dev/null +++ b/internal/metrics/batch.go @@ -0,0 +1,80 @@ +package metrics + +import "time" + +var batchResultBytesBuckets = []float64{1024, 4096, 16384, 65536, 262144, 1048576} + +var batchJobOutcomes = map[string]struct{}{ + "created": {}, "reused": {}, "done": {}, "partial": {}, + "failed": {}, "cancelled": {}, "restart": {}, +} + +var batchTargetOutcomes = map[string]struct{}{ + "ok": {}, "blocked": {}, "timeout": {}, "busy": {}, + "upstream_4xx": {}, "upstream_5xx": {}, "unsupported": {}, + "too_long": {}, "cancel": {}, +} + +// ObserveBatchJob records one batch outcome. Values outside the known set fold +// to "other" so a raw string never becomes a label. +func (s *Set) ObserveBatchJob(outcome string) { + if s == nil { + return + } + if _, ok := batchJobOutcomes[outcome]; !ok { + outcome = "other" + } + s.BatchJobsTotal.WithLabelValues(outcome).Inc() +} + +// ObserveBatchTarget records one target outcome. Values outside the known set +// fold to "other" so a raw string never becomes a label. +func (s *Set) ObserveBatchTarget(outcome string) { + if s == nil { + return + } + if _, ok := batchTargetOutcomes[outcome]; !ok { + outcome = "other" + } + s.BatchTargetsTotal.WithLabelValues(outcome).Inc() +} + +// SetBatchQueueDepth sets the live job count gauge (queued+running). +func (s *Set) SetBatchQueueDepth(n int) { + if s == nil { + return + } + s.BatchQueueDepth.Set(float64(n)) +} + +// SetBatchTargetsInflight sets the in-flight targets gauge. +func (s *Set) SetBatchTargetsInflight(n int) { + if s == nil { + return + } + s.BatchTargetsInflight.Set(float64(n)) +} + +// ObserveBatchJobSeconds records the wall time from submit to a batch's terminal status. +func (s *Set) ObserveBatchJobSeconds(d time.Duration) { + if s == nil { + return + } + s.BatchJobSeconds.Observe(d.Seconds()) +} + +// ObserveBatchTargetSeconds records the wall time for one target fetch. +func (s *Set) ObserveBatchTargetSeconds(d time.Duration) { + if s == nil { + return + } + s.BatchTargetSeconds.Observe(d.Seconds()) +} + +// ObserveBatchResultBytes records the stored byte count of one target result. +func (s *Set) ObserveBatchResultBytes(n int) { + if s == nil { + return + } + s.BatchResultBytes.Observe(float64(n)) +} diff --git a/internal/metrics/batch_test.go b/internal/metrics/batch_test.go new file mode 100644 index 0000000..8e1cad3 --- /dev/null +++ b/internal/metrics/batch_test.go @@ -0,0 +1,61 @@ +package metrics + +import ( + "io" + "net/http/httptest" + "strings" + "testing" + "time" +) + +type batchSrc struct{} + +func (batchSrc) Snapshot() Snapshot { return Snapshot{} } + +func TestBatchSeries(t *testing.T) { + s := New(batchSrc{}) + s.ObserveBatchJob("created") + s.ObserveBatchJob("done") + s.ObserveBatchJob("done") + s.ObserveBatchJob("reused") + s.ObserveBatchJob("some raw string") // folded to "other", never a label + s.ObserveBatchTarget("ok") + s.ObserveBatchTarget("ok") + s.ObserveBatchTarget("blocked") + s.ObserveBatchTarget("upstream_5xx") + s.ObserveBatchTarget("leak me") // folded to "other" + s.SetBatchQueueDepth(3) + s.SetBatchTargetsInflight(1) + s.ObserveBatchJobSeconds(4 * time.Second) + s.ObserveBatchTargetSeconds(1500 * time.Millisecond) + s.ObserveBatchResultBytes(4096) + + req := httptest.NewRequest("GET", "/metrics", nil) + req.RemoteAddr, req.Host = "127.0.0.1:9", "127.0.0.1:8890" + rec := httptest.NewRecorder() + s.Handler().ServeHTTP(rec, req) + body, _ := io.ReadAll(rec.Body) + out := string(body) + for _, want := range []string{ + `sift_batch_jobs_total{outcome="created"} 1`, + `sift_batch_jobs_total{outcome="done"} 2`, + `sift_batch_jobs_total{outcome="reused"} 1`, + `sift_batch_jobs_total{outcome="other"} 1`, + `sift_batch_targets_total{outcome="ok"} 2`, + `sift_batch_targets_total{outcome="blocked"} 1`, + `sift_batch_targets_total{outcome="upstream_5xx"} 1`, + `sift_batch_targets_total{outcome="other"} 1`, + `sift_batch_queue_depth 3`, + `sift_batch_targets_inflight 1`, + `sift_batch_job_seconds_count 1`, + `sift_batch_target_seconds_count 1`, + `sift_batch_result_bytes_count 1`, + } { + if !strings.Contains(out, want) { + t.Errorf("metrics lack %q", want) + } + } + if strings.Contains(out, "raw") || strings.Contains(out, "leak") { + t.Error("a raw outcome string leaked into a label") + } +} diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index 525edb2..e08dc0e 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -50,15 +50,22 @@ var pdfOutcomes = map[string]struct{}{ // Set is a private registry of sift_* series. type Set struct { - Reg *prometheus.Registry - Requests *prometheus.CounterVec - Duration *prometheus.HistogramVec - PDFTotal *prometheus.CounterVec - PDFSeconds prometheus.Histogram - PDFPages prometheus.Counter - PDFWait prometheus.Histogram - PDFCacheHits prometheus.Counter - handler http.Handler + Reg *prometheus.Registry + Requests *prometheus.CounterVec + Duration *prometheus.HistogramVec + PDFTotal *prometheus.CounterVec + PDFSeconds prometheus.Histogram + PDFPages prometheus.Counter + PDFWait prometheus.Histogram + PDFCacheHits prometheus.Counter + BatchJobsTotal *prometheus.CounterVec + BatchTargetsTotal *prometheus.CounterVec + BatchQueueDepth prometheus.Gauge + BatchTargetsInflight prometheus.Gauge + BatchJobSeconds prometheus.Histogram + BatchTargetSeconds prometheus.Histogram + BatchResultBytes prometheus.Histogram + handler http.Handler } func New(src Source) *Set { @@ -94,17 +101,57 @@ func New(src Source) *Set { Name: "sift_pdf_cache_hits_total", Help: "PDF answers served from the document cache.", }) - reg.MustRegister(requests, duration, pdfTotal, pdfSeconds, pdfPages, pdfWait, pdfCacheHits, &collector{src: src}) + batchJobs := prometheus.NewCounterVec(prometheus.CounterOpts{ + Name: "sift_batch_jobs_total", + Help: "Batch jobs by outcome.", + }, []string{"outcome"}) + batchTargets := prometheus.NewCounterVec(prometheus.CounterOpts{ + Name: "sift_batch_targets_total", + Help: "Batch targets by outcome.", + }, []string{"outcome"}) + batchQueueDepth := prometheus.NewGauge(prometheus.GaugeOpts{ + Name: "sift_batch_queue_depth", + Help: "Live batch jobs (queued+running).", + }) + batchTargetsInflight := prometheus.NewGauge(prometheus.GaugeOpts{ + Name: "sift_batch_targets_inflight", + Help: "Batch targets currently being fetched.", + }) + batchJobSeconds := prometheus.NewHistogram(prometheus.HistogramOpts{ + Name: "sift_batch_job_seconds", + Help: "Batch wall time from submit to terminal status.", + Buckets: durationBuckets, + }) + batchTargetSeconds := prometheus.NewHistogram(prometheus.HistogramOpts{ + Name: "sift_batch_target_seconds", + Help: "Batch target fetch wall time.", + Buckets: durationBuckets, + }) + batchResultBytes := prometheus.NewHistogram(prometheus.HistogramOpts{ + Name: "sift_batch_result_bytes", + Help: "Stored bytes per batch target result.", + Buckets: batchResultBytesBuckets, + }) + reg.MustRegister(requests, duration, pdfTotal, pdfSeconds, pdfPages, pdfWait, pdfCacheHits, + batchJobs, batchTargets, batchQueueDepth, batchTargetsInflight, + batchJobSeconds, batchTargetSeconds, batchResultBytes, &collector{src: src}) return &Set{ - Reg: reg, - Requests: requests, - Duration: duration, - PDFTotal: pdfTotal, - PDFSeconds: pdfSeconds, - PDFPages: pdfPages, - PDFWait: pdfWait, - PDFCacheHits: pdfCacheHits, - handler: promhttp.HandlerFor(reg, promhttp.HandlerOpts{EnableOpenMetrics: false}), + Reg: reg, + Requests: requests, + Duration: duration, + PDFTotal: pdfTotal, + PDFSeconds: pdfSeconds, + PDFPages: pdfPages, + PDFWait: pdfWait, + PDFCacheHits: pdfCacheHits, + BatchJobsTotal: batchJobs, + BatchTargetsTotal: batchTargets, + BatchQueueDepth: batchQueueDepth, + BatchTargetsInflight: batchTargetsInflight, + BatchJobSeconds: batchJobSeconds, + BatchTargetSeconds: batchTargetSeconds, + BatchResultBytes: batchResultBytes, + handler: promhttp.HandlerFor(reg, promhttp.HandlerOpts{EnableOpenMetrics: false}), } } diff --git a/skill/SKILL.md b/skill/SKILL.md index 1b3c381..77d5391 100644 --- a/skill/SKILL.md +++ b/skill/SKILL.md @@ -55,6 +55,33 @@ Useful flags: `--mode digest|clean|raw` (default `digest` when `--question` is g else `clean`), `--no-context` (skip Honcho lookup), `--recover` (server-side archive ladder for blocked pages), `--json` (full envelope: content, source, context, sizes, warnings). +## Batch + +When you will read the content of several URLs later (a shortlist to compare), submit them once +instead of a loop of `sift fetch`: + +```bash +# one JSON object, or a bare array of URLs +sift batch submit targets.json +# ["https://a", "https://b"] also works; each string becomes {"url": ...} +``` + +Then poll and read: + +```bash +sift batch status # summary: per-target status, no content +sift batch status --results # everything at once — can be large, avoid unless needed +sift batch get # one target's result, as `sift fetch --json` +sift batch cancel # stop a queued job +``` + +- Submit once; a retry of the same list from this caller returns the same job (`reused`), it does + not start a second. +- Don't use `--results` unless you need every result at once — a batch of 32 can be several + megabytes. Read each piece with `sift batch get `. +- A batch is slow on purpose: it runs one target at a time per job. For one URL you need now, use + `sift fetch`. + ## PDFs PDFs work: `sift fetch` extracts them and returns the text. A short one comes back whole; a