Repository navigation
Conversation
|
Review complete. 🟡 1 medium 💬 Inline comments (1)
The PR adds a build lifecycle coordinator that permits, queues, watchdogs, and drains Graphile schema builds, then threads the shared coordinator through the cache, middleware, and server so builds are fenced before shutdown and bounded under load. A single high-confidence issue surfaced: a routine disposal failure now latches the coordinator into a permanent
Reviewed commit: 51f923f |
![tenki-reviewer[bot]](https://avatars.githubusercontent.com/in/2900641?s=60&v=4){kind=small}
There was a problem hiding this comment.
This change introduces a GraphileBuildCoordinator that serializes, bounds, and drains Graphile schema builds across the graphile-cache, GraphQL server, and explorer, with a new error-code registry and env config plumbing.
Key findings
- 🟡 Disposal failure permanently bricks build coordination — graphile-cache.ts:276
Consolidate PR #1747, including its merge-owned release instrumentation, before rebasing onto current main.
031a1b0 to
ab590b1
Compare
Distinct concurrent cold schema misses now enter one process-wide FIFO coordinator for cached Server and Explorer producers. One active permit covers admission, eviction/disposal waits, allocation, readiness, publication and cleanup; the waiting queue defaults to 16. Exact-key calls share a flight.
Rebased onto main
299b3920; tested headab590b19. This main-based PR includes F04/F17/F19/F20 prerequisites, including #1853's commits. Both PRs target main, but their diffs overlap.Measured conclusion: serialization and queueing work. This workload shows no stable general speedup or meaningful RSS reduction; the demonstrated value is controlling concurrent cold builds and accepting a burst through a bounded queue.
The watchdog (default 300000 ms) fences late publication without pretending to cancel JavaScript or releasing the permit before actual work settles. Watchdog expiry, disposal failure and drain timeout permanently latch the coordinator unavailable until restart. Shutdown fences before HTTP close and uses one drain deadline (default 30000 ms); pools stay open while actual build work remains. Main's existing measured heap budget remains enforced by the admitted path. The uncached single-instance dev server and full request/WebSocket retirement are outside this change.
Measured medians, three runs per row; elapsed time completes all requested builds and validation reads:
With 8 cold keys, main/F19/F21 peak concurrent creates were 8/8/1 at loose capacity and 8/4/1 at tight capacity. F21 queued up to 7 and completed all 8 initial calls without capacity rejection. F19 at tight capacity completed 4 initial calls and rejected 4 before allocation; one immediate in-process retry wave after the initial wave settled completed the remainder.
All-completed timing includes initial rejection time, waiting for that wave, retries and scheduling, starting before the initial 8 calls. No client/network/backoff delay is added; real retry latency may be higher. RSS includes every retry wave.
F21's loose-8 median was 7.3% lower than main, but loose-4 was 15.5% higher. Loose-8 ranges overlap: main 6580–7810 ms, F19 6100–7496 ms, F21 6240–7610 ms. These samples do not justify a general speedup claim; RSS was essentially unchanged or higher.
Measured on 2026-11-04: Node 24.20.0, PostgreSQL 18.6, 3 available CPUs, approximately 4 GiB RAM,
--expose-gc --max-old-space-size=768. Each fresh worker builds a 24-table schema and validates a seeded GraphQL read. Three repetitions per arm/scenario, randomized sequential order; 36/36 runs passed across main/F19/F21. Every successful build produced the same schema hash and query result.Loose capacity is 50 residents; tight capacity is 4. Both measured resident heap budget and process heap watermark are set to 4 GiB, with a 64 MiB build reserve, to isolate count/coordinator behavior. This does not test default heap-watermark tuning or OOM prevention. No subscriptions are used.
The validation read runs inside the create callback before publication to prevent eviction racing validation. Consequently, permit occupancy includes query time: this is a cache-layer cold build-and-query microbenchmark, not production HTTP latency/throughput or a bound on all SQL concurrency. RSS peaks cover the entire run, including retry waves.
Validation at the rebased head:
Raw measurements, samples and runtime/source provenance are retained in the task workspace at
/workspace/scratch/admission-rebase/bench/results-24.json; grouped medians/ranges are inresults-24-summary.json, alongside the worker, runner and analysis scripts.