Repository navigation
Conversation
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.
Cache keys can be reused while an older Graphile instance is still releasing. Cleanup now follows the exact
GraphileCacheEntryobject, so replacing an entry releases both generations and repeated cleanup of one generation returns the same promise, including after failure.Stacked on #1881 (
fix/graphile-readiness-disposal, head49ae6daf45f8b024d374f8f207ace67c0fe9fa81). This PR contains only the disposal follow-up. The source reference is #1747 at89f456161e6c5558bd1fd41db315d27f02ab79a1; that PR and its downstream stack are unchanged.Changes
disposeUncachedEntrythrough identity-basedscheduleDisposal. Register the promise before resource callbacks, including synchronous reentrant calls.utils/preset-services.ts. Release them once in reverse order, await their public return values, continue after individual errors, and preserve single errors or aggregate multiple errors.clearGraphileCache,waitForActiveDisposals,waitForEntryDisposal, anddisposeUncachedEntry. A bulk drain snapshots generations scheduled before the call, retains settled failures until that snapshot is acknowledged, and lets concurrent drains each observe their captured failures. Later generations belong to the next drain. Exact-entry promises retain their outcome after acknowledgement; an unscheduled entry still returns a resolvedPromise<void>.closeAllCachesshares concurrent work and attempts pg-cache pool cleanup even if instance cleanup fails, preserving errors from both phases.schemaResult, sequential awaits, deferredcreateServ, void contract, and utils layout.start()so partial startup is stopped.Ownership and limits
Each factory invocation owns its own HTTP server and fresh preset service objects; it never receives or closes the shared main HTTP server. Preset service release honors the upstream ownership contract: externally supplied pools and subscribers remain caller-owned. Only
closeAllCachesseparately asks pg-cache to close its managed pools.Disposal completion means the public release calls have settled. It does not guarantee that upstream asynchronous UNLISTEN/client return, listener cleanup, or watch/background work has fully drained. PostGraphile/Grafserv may handle some release errors internally, and pg-cache currently settles its own pool-disposal errors internally; this PR preserves errors those public APIs expose. It does not repair the watch boundaries documented in #1881, patch dependencies, change realtime subscription internals, or import the old performance harness.
Validation
CNC, Node 22.23.3 / pnpm 10.34.5 / PostgreSQL 18.6:
pnpm --filter graphile-cache test --runInBand: 8 suites / 78 tests passed. 74 are database-free, including the inherited nine strict Node subprocess cases using real locked packages. Four use real PostgreSQL throughpgsql-test.graphile-single-flight.test.tsandgraphile-introspection.test.ts: 2 suites / 11 tests passed.tsc --noEmitchecks, package ESLint (only the existing unusedunregisterwarning),git diff --check, frozen offline dependency installation, supply-chain policy, and CI test-registration check passed.pgsql-test. No task test databases remain.GitHub checks at the published head
At
1fc1d5442d41289ffbf6b45f1388143315994628, no GitHub Actions build/test workflow was triggered: the repository's PR workflow targets onlymainandv1, while this stacked PR targetsfix/graphile-readiness-disposal. Socket Security Project Report completed successfully. Socket PR Alerts also reports a successful check conclusion, but its report is explicitly skipped because it found no net dependency changes. These checks do not constitute a build/test CI pass. This PR remains a Draft.