port(v2): partition-parallel drain for large sales-filter backlogs (#71 on the v2 line)#72
Merged
robrigo merged 4 commits intoJun 10, 2026
Conversation
…backlogs A deep catchup or bulk load leaves atomicmarket_sales_filters_updates with a multi-million-row backlog that the (correctly) single-flight drain clears at random-read I/O speed - days to weeks of stale /v2/sales filters. The recompute parallelizes perfectly across disjoint sales, so: * update_atomicmarket_sales_filters_partition(part_count, part_index, batch): the 1.7.11 drain restricted to sale rows with sale_id % part_count = part_index. Recompute CTE block is byte-for-byte 1.7.11. Workers take the stock drain's advisory key SHARED (stock takes it exclusively - unchanged), plus an exclusive per-partition sub-lock: N workers coexist, never with the stock drain, duplicates no-op. Nothing runs unless an operator launches it. * normalize_atomicmarket_sales_filters_offers(batch): converts queued offer rows into the equivalent queued sale rows (same resolution the stock drain does per batch) so workers can own them; 1.7.11 seq-guard protocol. * index on atomicmarket_sales_filters (assets_contract, offer_id): the drain's offer->sales resolution was a hash join scanning the multi-GB filter partitions every batch containing offer rows; now index probes. Also speeds the stock drain. * bin/drain-sales-filters.js [--partitions N] [--batch B]: operator runner - normalizes offers, runs N partition workers to empty, ANALYZEs after. Validated against a production WAX dataset (3.9M-row backlog).
…tionale Review follow-ups on atomicassets#71 (comment-only, no behavior change): - Replace the loose 'pre-build under the same names' note with the actual ON ONLY + CREATE INDEX CONCURRENTLY + ATTACH PARTITION recipe (attachment matches on definition equivalence, not names) as used for the production validation run. - State the plain-temp-table rationale (1.7.11 precedent: reused pool connections, no DROP-IF-EXISTS notice spam; the claimed (key,seq) set must survive across statements so a CTE chain cannot replace it; per-call DDL is negligible at drain cadence).
….0.1 The v2 line's migration chain is at 2.0.0 (rc tags already initialize databases at that version), so the main-line 1.7.13 directory would sort before 2.0.0 and never apply to them. Renumbered to 2.0.1; SQL content unchanged from PR atomicassets#71.
igorls
marked this pull request as ready for review
June 10, 2026 04:08
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds an opt-in, partition-parallel backlog drain for atomicmarket_sales_filters_updates, including the supporting DB functions/index, a CLI runner, and integration tests validating partition scoping, parity with the stock drain, and advisory-lock behavior.
Changes:
- Add migration
2.0.1implementing offer normalization + partition worker drain functions and an index to speed offer→sale resolution. - Add a
drain-sales-filtersCLI script to run parallel workers safely alongside the live filler. - Add integration tests covering correctness (parity), partition isolation, and lock protocol across multiple connections.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/filler/handlers/atomicmarket/sales-filters-parallel-drain.integration.test.ts | New integration coverage for partition drain, normalization, and advisory-lock protocol. |
| src/bin/drain-sales-filters.ts | New CLI script orchestrating offer normalization + parallel partition workers + ANALYZE. |
| definitions/migrations/2.0.1/database.sql | Bumps DB version to 2.0.1 with migration context. |
| definitions/migrations/2.0.1/atomicmarket.sql | Adds offer-id index plus normalize_* and update_*_partition functions with locking protocol. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+69
to
+73
| const a = await client.createFullSale(); | ||
| const b = await client.createFullSale({ sale_id: Number(a.sale_id) + 1 }); // adjacent => opposite parity | ||
|
|
||
| const mine = Number(a.sale_id) % 2; | ||
| await client.query('SELECT update_atomicmarket_sales_filters_partition(2, $1, 5000)', [mine]); |
Comment on lines
+68
to
+69
| SET LOCAL statement_timeout = 0; | ||
| SET LOCAL lock_timeout = '60s'; |
robrigo
reviewed
Jun 10, 2026
robrigo
left a comment
robrigo
left a comment
Contributor
There was a problem hiding this comment.
Review (FACINGS) — LGTM.
- Port fidelity verified mechanically: normalized diff-of-diffs vs #71 shows the only differences are the port-note comments; SQL, runner, and tests are content-identical.
2.0.1numbering is exactly right for this line — it sorts after2.0.0, so databases already initialized from the 2.0.0-rc tags (jungle4 is atdbinfo.version = 2.0.0today) will still pick it up. A1.7.13here would have been silently skipped on those DBs.- Ran on PostgreSQL 18 against this branch: full v2-line integration 349 passing (340 + the 9 new parallel-drain tests, running under the 2.0.1 banner), typecheck clean.
Heads-up, not a blocker: this PR gets no CI — ci.yaml only triggers on PRs to main, and this targets feat/atomicassets-v2. The local run above is currently the only gate. Worth adding feat/atomicassets-v2 to the pull_request.branches list (here or as a follow-up) so the v2 line gets PR CI.
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
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.
Port of #71 onto
feat/atomicassets-v2, so the 2.0.0 line ships with a recovery path for bulkatomicmarket_sales_filters_updatesbacklogs (deep catchup, bulk load, drain outage). Opened separately per the cherry-pick-to-v2 workflow used for #68/#69/#70 in 2.0.0-rc2 — happy to close either side if you prefer a different route.Differences from #71 (content otherwise unchanged)
1.7.13→2.0.1so it sorts after2.0.0and still applies to databases already initialized from the2.0.0-rctags.mainis v1.7.11, its1.7.11migration is byte-identical to main's, and the2.0.0migration does not touch the drain — so the parity guarantee (recompute CTE byte-for-byte 1.7.11) holds identically here.Validation
to_jsonbequality, partition scoping, offer normalization, and the advisory-lock matrix on real concurrent backends).(assets_contract, offer_id)index; reader at head throughout. Details in feat(filler): opt-in partition-parallel drain for large sales-filter backlogs #71.movesales-filter drain baseline item: if v2 ends up enqueueing move-driven changes into the same queue, the partition workers consume them transparently once they resolve to sale rows.