ci(preview): bisect invalid seed rows instead of replaying batches row-by-row (stage 3) - #1151
Draft
kyle-compute wants to merge 1 commit into
Draft
ci(preview): bisect invalid seed rows instead of replaying batches row-by-row (stage 3)#1151kyle-compute wants to merge 1 commit into
kyle-compute wants to merge 1 commit into
Conversation
…w-by-row The #1139 seed pathology: one constraint-violating row in a multi-row upsert batch dropped the whole chunk into a row-by-row fallback, so five legacy rows among ~900-row chunks cost thousands of pooler round trips (318s). The fallback now rolls the failed batch back at its savepoint, splits it in half, and recurses only into failing halves until invalid rows stand alone: O(k log n) probes for k bad rows in n rows, with valid rows retained. Only IntegrityError (SQLSTATE class 23) is splittable. The old catch-all also swallowed DBAPIError, so a dead connection or serialization failure was silently recorded as skipped rows -- transient infrastructure errors now abort the load instead of masquerading as bad data. Every seed run emits a deterministic report (batches attempted/split, rows skipped, SQLSTATE + constraint names with table-qualified row ids, before/after row counts by table) that flows through a new prepare-job output into the timing artifact and the job summary. Also repairs the local seed-test fixtures, which had silently rotted (they skip without ODDISH_DATABASE_URL): SQLAlchemy compiles multi-row inserts from the first dict's keys, so sparse rows had extra keys silently dropped and missing keys hard-error; a _rows() helper now unions the keys of each heterogeneous fixture group.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Oddish previewCommit:
Vercel deployment URL: https://oddish-meg535pol.oddish.app Plan:
This comment is updated by the PR Preview workflow. |
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.
Stacked on #1144 (
ci/preview-fencing). This diff contains only stage-3 changes; GitHub retargets tostagingwhen the chain merges. Opened as draft: merge only after the earlier stages land.Problem
#1139's 318s seed: one constraint-violating row in a multi-row upsert batch dropped the whole chunk into a row-by-row fallback, so five legacy rows among ~900-row chunks cost thousands of pooler round trips. Worse, the fallback caught
DBAPIError, so a dead connection or serialization failure was silently recorded as "skipped rows" — transient infrastructure failure masquerading as bad data.The plan's second seed concern (migration/seed ordering) is already enforced on staging: the bootstrap's trust marker folds in a migration-content fingerprint, so an edited or added migration invalidates the cached schema and forces a full rebuild, and
_rebuildseeds beforealembic upgrade head— pinned bytest_rebuild_snapshots_prod_before_touching_the_branch(seedprecedesupgrade:oddish/upgrade:backend, trust marker last). No code change needed there; this PR pins the remaining gap, the loader.What changes
backend/preview_seed.py— the row-by-row fallback is replaced by_isolate_bad_rows, a divide-and-conquer driver:k bad rows in an n-row batch: O(k log n) round trips instead of O(n). Only
IntegrityError(SQLSTATE class 23) is splittable; network/timeout/serialization errors propagate and fail the seed loudly. The recursion is the only fallback path — the old batch-then-row-by-row two-step is gone.Every
seed()run now returns a deterministic report:batches_attempted,batches_split,rows_skipped, skip causes keyed bySQLSTATE+ constraint name with table-qualified row ids (sorted, capped at 100/cause), and before/after row counts per drawn table (oneUNION ALLround trip — per-table COUNTs would each pay a NullPool pooler handshake).Emission path:
seed_preview_db.pywrites the report as single-line JSON to$PREVIEW_SEED_STATS_FILE→prepare_preview_database.shforwards it as theseed_statsjob output and prints a summary line → the gate'sSEED_STATSenv →record_preview_metrics.pyfolds it into the timing artifact underseed. Malformed payloads degrade to an empty section; telemetry never fails the deploy.Fixture repair (
test_preview_seed.py): the DB-backed suite had silently rotted — it skips withoutODDISH_DATABASE_URL, so drift accumulated: SQLAlchemy compiles a multi-row insert from the first dict's keys, so sparse rows had extra keys (e.g.t-del'sdeleted_at) silently dropped and missing keys (created_by_user_id) hard-errored. A_rows()helper now unions the keys of each heterogeneous fixture group.Validation
Pure unit tests (
test_preview_seed_isolation.py, no DB) — the plan's enumerated scenarios:task-9:exp-3key formatOperationalError→ propagates, zero skips recordedDB-backed suite (
test_preview_seed.py, local Postgres 16 container): updated the bulk-fallback test — a colliding task now correctly cascades to itstask_versions/task_experimentschildren, and the report pins all three skips with their constraint identities. 104 tests pass, plus an end-to-endseed_preview_db.pysmoke (sampled 36 rows, isolated a real unique-violation skip in 0.5s, wrote the stats JSON).The ordering invariant (item A) is pinned by the existing bootstrap orchestration tests — 21 pass.
Risk
Rollback
Revert this PR. The stats output is additive; the artifact tolerates its absence (
seed: {}).Acceptance criteria