fix(sketch-db): POST /api/v1/db/backfill uses create_checked (§10.5) - #43
Merged
Merged
Conversation
The HTTP handler was calling `BackfillRegistry::create()`, which skips every invariant. PR #40 added `create_checked()` — the same entry point the `BackfillService` uses — but the HTTP surface kept the unchecked path, so controllers could file jobs against unknown agg_ids or time ranges that overlap live ingest / fall outside retention, and the worker would only discover the mistake later (or silently waste I/O on soon-to-be-evicted windows). Changes: - `handle_post_backfill_job` now requires both the backfill registry AND the schema registry (503 if either is absent), calls `create_checked(&schemas, ..., data_retention_ms)`, and maps `CreateError` to distinct HTTP statuses: 404 for `UnknownAgg`, 409 for `Overlap` / `OutOfRetention`, 201 on success. 400 on malformed body / inverted range is preserved. - `HttpServer::with_data_retention_ms(u64)` threads the SimpleMapStore data-retention horizon into AppState so Method B rejects stale ranges at create time. `main.rs` wires it from `--persistence-delete-older-than-secs` when > 0. - Test helper renamed `setup_test_server_with_backfill_and_schemas` and now takes an `&[u64]` of agg_ids to pre-register as Active. All four existing backfill tests updated. - Two new tests exercise the new error paths: `test_backfill_post_unknown_agg_returns_404` and `test_backfill_post_overlap_with_live_ingest_returns_409`. 725 lib tests pass (was 723), clippy clean, fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The HTTP endpoint
POST /api/v1/db/backfillwas callingBackfillRegistry::create()— the unchecked path. PR #40 addedcreate_checked()as the authoritative entry point with three §10.5 invariants (known agg, time-disjoint, within-retention), but the HTTP surface kept routing throughcreate(). So controllers could file jobs against unknown agg_ids or time ranges that overlap live ingest / fall outside the SimpleMapStore retention horizon, and the worker would either fail later or silently waste I/O on soon-to-be-evicted windows.What's in this PR
handle_post_backfill_jobnow requires both the backfill registry and the schema registry (503 on either missing), callscreate_checked(&schemas, ..., data_retention_ms), and mapsCreateErrorto distinct HTTP statuses:UnknownAgg→ 404Overlap/OutOfRetention→ 409HttpServer::with_data_retention_ms(u64)threads the SimpleMapStore data-retention horizon intoAppState, so Method B can reject stale ranges at create time.main.rswires it from--persistence-delete-older-than-secswhen > 0 (unset → retention precheck skipped, remaining two invariants still enforced).setup_test_server_with_backfill_and_schemas, takes&[u64]of agg_ids to pre-register as Active. Four existing backfill tests updated.Why
e2e on the precompute stack (the one that caught #42) showed that the HTTP endpoint accepted a job against a known agg with
end_ms>created_at_ms— a clear §10.5 overlap — and returned 201 Created. With this PR it now returns 409 Conflict, matching the invariant the design doc already describes.Test plan
cargo test -p query_engine_rust --lib— 725 pass (was 723; +2 new tests)cargo clippy --all-targets -- -D warnings— cleancargo fmt --all -- --check— cleantest_backfill_post_unknown_agg_returns_404test_backfill_post_overlap_with_live_ingest_returns_409🤖 Generated with Claude Code