Skip to content

Add Graphile readiness and generation-scoped disposal - #1747

Open
Zetazzz wants to merge 5 commits into
mainfrom
agent/graphile-readiness-disposal
Open

Zetazzz wants to merge 5 commits into
mainfrom
agent/graphile-readiness-disposal

Conversation

@Zetazzz

@Zetazzz Zetazzz commented Aug 16, 2026 •

Copy link
Copy Markdown
Contributor

Graphile creation waits for schema gathering and Grafserv readiness. Failed builds release their owned resources, and cache disposal is coalesced by the exact entry object so an old generation cannot suppress cleanup of a replacement using the same key.

Preset services are released after PostGraphile teardown using unmodified @dataplan/pg 1.1.1. Disposal succeeds when the public release calls return successfully, including promises exposed by those APIs; it does not promise that upstream background UNLISTEN/client return has already completed. The dataplan patch, its declarations, and PNPM patch references are removed. Idempotency and public release failure propagation remain covered.

Cleanup failures now remain observable after background disposal settles. Bulk drains snapshot the generations scheduled before the call, preserve the original single failure, aggregate multiple resource failures, and acknowledge only that snapshot. Concurrent drains each observe their captured failures. Exact-entry release promises remain rejected after acknowledgement. Failed-build cleanup preserves the build error as its cause, realtime startup failures dispose their generation, and pool cleanup is attempted even when a resident release rejects.

A TypeScript connection-lifecycle suite now uses packages/perf-harness and pgsql-test for idle services, active subscriptions, startup/release races, Graphile build/dispose, subscribed disposal, failed builds, and same-key generation replacement. It defaults to a low-resource functional run: one repetition, concurrency 1, pool limit 2, and 72 lifecycles. Larger stress runs are opt-in and documented. All modes measure actual public release calls, including replacement and failed-build cleanup. Generated reports and logs are not committed.

Earlier baseline validation on 2752c568 (Node 24.20.0 / PostgreSQL 18.6):

  • All 7 low-resource cases passed in separate processes. Every case returned checked-out connections and queued acquisitions to zero, removed LISTEN channels, and returned database backends to the two-fixture-client baseline before fixture teardown/process exit.
  • Startup/release churn briefly queued 18 acquisitions and settled within about 112ms in the final run. This is asynchronous cleanup/queueing, not a persistent checked-out connection leak in this run.
  • The unpatched subscriber does retain client event listeners: 20 subscription cycles left up to 10 notification listeners and 10 error listeners per reused client, with 2 listener-limit warnings. This is reported separately; no dependency workaround is introduced here. Long-duration/load severity remains unmeasured; the larger run was stopped to respect local resource limits.
  • Graphile-cache: 4 suites / 19 tests passed; perf-harness: 11 suites / 100 tests verified (99 passed initially; one existing CLI test exceeded the local 5s timeout under contention and passed a targeted retry with a 30s limit). CJS/ESM builds, focused lint, frozen installation, and supply-chain policy checks passed.

Earlier cloud validation: CI tests run 35623188188 passed all 17 jobs on commit 2752c568, including the Graphile PostgreSQL tests and Windows checks. graphile-cache remains in the PostgreSQL CI matrix.

Refs constructive-io/constructive-planning#1693 (F04).

CNC follow-up validation: focused readiness/disposal/resource-release tests passed; the final dependent stack passes 96 graphile-cache tests. CJS/ESM builds, frozen-lockfile installation, dependency policy, and test registration checks passed. The earlier stress observations above are historical evidence; this follow-up does not rerun the extended performance suite.

@Zetazzz
Zetazzz force-pushed the agent/graphile-readiness-disposal branch from dec013c to e77d653 Compare September 6, 2026 01:51
@Zetazzz
Zetazzz force-pushed the test/performance-harness-core branch 2 times, most recently from 658736e to f7c63ec Compare September 7, 2026 03:22
@Zetazzz Zetazzz changed the title Add Graphile build readiness and awaited disposal Add Graphile readiness and generation-scoped disposal Sep 21, 2026
@Zetazzz
Zetazzz marked this pull request as ready for review September 21, 2026 15:47
@tenki-reviewer

tenki-reviewer Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Review complete. 🟡 1 medium

💬 Inline comments (1)

  • 🟡 Replacement/failed-build modes skip real release accounting — worker.ts:200

This PR reworks graphile/graphile-cache service lifecycle semantics.

Files Change
graphile-cache/src/{preset-services,create-instance,build-readiness,graphile-cache,index}.ts Introduces preset service management, a generation-replacement disposal model, build-readiness gating, and cache-wide disposal/close helpers.
graphile-cache/src/__tests__/* Adds lifecycle, disposal, build-readiness, and service-release unit tests using the pgsql-test harness.
perf-harness/src/connection-lifecycle/{suite,worker}.ts New load suite exercising concurrent cache-generations with leak/channel detection and latency metering.
graphile-cache/jest.config.js, package.json, .github/workflows/run-tests.yaml Wires the new package into CI with a ts-jest transform and workspace dev-dependencies.

The change is well-structured and well-tested; the main concern is that the perf-harness metering in replacement/failed-build modes reports placeholder release accounting rather than real disposals, which caps the harness's ability to detect leaks on those paths.

Reviewed commit: 1c9dfb1

@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.

Adds caching-instance and service-lifecycle logic to graphile-cache (preset services, build readiness, disposal) plus a perf-harness connection-lifecycle suite, with new CI wiring.

Key findings

  • 🟡 Replacement/failed-build modes skip real release accounting — worker.ts:200

Comment thread packages/perf-harness/src/connection-lifecycle/worker.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