Skip to content

F21: bound and serialize Graphile builds with watchdog and drain - #1855

Open
Zetazzz wants to merge 4 commits into
graphile-build-coalescing-c8f5bfd9from
graphile-build-coordinator-c8f5bfd9
Open

Zetazzz wants to merge 4 commits into
graphile-build-coalescing-c8f5bfd9from
graphile-build-coordinator-c8f5bfd9

Conversation

@Zetazzz

@Zetazzz Zetazzz commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Implements F21 from constructive-io/constructive-planning#1693, stacked on F20 (#1854) and F19 (#1853), above F17 (#1852) and F04 (#1747).

Concurrent distinct schema misses now enter one process-wide FIFO coordinator before capacity admission. One active permit covers eviction waits, preset allocation, schema/readiness work, publication, and cleanup; the waiting queue defaults to 16. Exact-key joins consume no additional slot. A synchronous admission check rejects overflow before allocating flight records, including a burst of 100 distinct calls in one JavaScript turn.

The watchdog (default 300000 ms) fences publication and promptly rejects active/queued callers while retaining the permit and any reservation until actual work and cleanup settle. Watchdog expiry, failed cleanup, and drain timeout permanently latch the coordinator unavailable. JavaScript work cannot be cancelled safely here; recovery requires process restart. A disappearing cache/disposal record does not prove a failed release reclaimed its resources.

Shutdown fences builds before HTTP close waits on those callers. One configurable deadline (default 30000 ms) covers actual coordinator work and flight settlement. A false drain result becomes a canonical timeout error, and pools are not closed. Once actual builds drain, resident cleanup failures remain observable and pool cleanup is still attempted; failures from both owners are aggregated. Healthy closed coordinators reopen only after the prior tasks and preparation scopes drain. Full request/WebSocket retirement remains outside this build-coordination slice.

Typed graphile.build options and strict GRAPHILE_BUILD_* environment overrides expose queueMax/watchdogMs/shutdownTimeoutMs. Node timer overflow values are rejected; absent overrides preserve configuration and do not claim explicit policy ownership.

The stack reuses the request-context owner for API/pool/transaction settings and the canonical error owner for causes, codes, and HTTP status. Build refusals retain their 503 response; unknown failures expose a generic 500. Cache environment parsing and precedence belong to graphql-env, and the existing LRU enforces the resolved runtime limits.

Validation on the updated stack:

  • 96 graphile-cache tests, including real PostgreSQL subscriber release, disposal failure snapshots, runtime capacity above 50, TTL expiry, exact-flight invalidation, watchdog cleanup, and shutdown timeout without pool close.
  • 109 graphql-env tests and 1 snapshot; 52 error-owner tests; 130 express-context tests; 33 focused server tests.
  • Seven real PostgreSQL/scoped-routing HTTP scenarios cover seeded queries, coalescing, failed-build retry and safe errors, authenticated/invalid flushes, health bypass, and canonical 503 after build shutdown. These replace mocked middleware tests and use the repository fixture and transaction hooks.
  • CJS/ESM builds passed for errors, express-context, graphql-types/env, graphile-cache, server, and Explorer. Frozen-lockfile installation, dependency policy, CI test registration, and diff checks passed. Focused lint reports no errors (import-order/unused-variable warnings remain).

Extended performance stress tests were not repeated in this follow-up; #1747 records the earlier measurements and upstream subscriber-listener limitation. The changes preserve public release boundaries and do not claim immediate completion of upstream background cleanup.

All newly authored commits use the requested personal Git author and committer identity. Existing PR branches are updated with fast-forward merges and retain the dependency stack.

@tenki-reviewer

tenki-reviewer Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

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 stuck state, bricking all future builds for the process lifetime. The coordinator core (permit/queue/watchdog/drain) is otherwise careful and well-tested; two lower-confidence shutdown-path concerns (deadline throw skipping pool close, shared-coordinator reconfigure) were reviewed but not retained.

Files Change
graphile/graphile-cache/src/build-coordinator.ts, build-flights.ts, admitted-build.ts, graphile-cache.ts, index.ts Add the coordinator state machine and wire it into build admission, disposal, and cache lifecycle.
graphile/graphile-cache/src/__tests__/* Add concurrency and fake-timer tests for coordinated build/admission/drain paths.
graphql/env/src/*, graphql/types/src/* Extend env/config merge and validation for the new build-coordination settings.
graphql/server/src/server.ts, middleware/graphile.ts, graphql/explorer/src/server.ts Fence builds before shutdown and configure the shared coordinator from server options.
packages/errors/src/registry.ts Register new lifecycle error codes (SCHEMA_BUILD_STUCK, QUEUE_FULL, DRAIN_TIMEOUT) and test coverage.

Reviewed commit: 51f923f

@tenki-reviewer tenki-reviewer Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread graphile/graphile-cache/src/graphile-cache.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant