Skip to content

fix(graphql): route makePgService pools through pg-cache in explorer and server - #861

Merged
pyramation merged 1 commit into
mainfrom
devin/1774038000-fix-untracked-makepgservice-pools
Mar 20, 2026
Merged

pyramation merged 1 commit into
mainfrom
devin/1774038000-fix-untracked-makepgservice-pools

Conversation

@pyramation

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #860. Applies the same fix to two more call sites where makePgService({ connectionString }) created PostGraphile-internal pools invisible to pg-cache:

  • graphql/explorer/src/server.ts — the GraphQL Explorer's dynamic schema introspection
  • graphql/server/src/middleware/graphile.ts — the main GraphQL server middleware (buildPreset)

Both now use getPgPool(pgConfig) to obtain a pg-cache-tracked pool and pass it via makePgService({ pool }).

A third instance in graphql/query/src/executor.ts was identified but intentionally left out: it has no external consumers, doesn't depend on pg-cache, and uses a different lifecycle (postgraphile() + pgl.release()).

Review & Testing Checklist for Human

  • Verify makePgService({ pool }) behaves identically to makePgService({ connectionString }) — PostGraphile's @dataplan/pg adaptor accepts both, but confirm there are no subtle differences in pool configuration, connection limits, or adaptor behavior (e.g., does PostGraphile call pool.end() on release when it doesn't own the pool?).
  • Pool sharing implications — getPgPool caches by database name. Multiple PostGraphile instances hitting the same database (e.g., different schemas in the explorer, or multiple tenants in the server) will now share a single pool. Previously each makePgService({ connectionString }) call created an independent pool. Verify this doesn't cause connection starvation under concurrent load.
  • Test the explorer: navigate to a database/schema in the GraphQL Explorer and confirm introspection + GraphiQL still work.
  • Test the server: confirm the main GraphQL API endpoint still serves queries correctly for at least one tenant.

Notes

  • The buildPreset signature in graphile.ts changed its first parameter from connectionString: string to pool: import('pg').Pool. This is internal-only (not exported).
  • The buildConnectionString import is removed from both files since it's no longer needed.

Link to Devin session: https://app.devin.ai/sessions/a8cbad4ea6f2434b87fc29ac082105fd
Requested by: @pyramation

…and server

Same pattern as the buildSchemaSDL fix (PR #860): makePgService({ connectionString })
creates internal pools invisible to pg-cache. Route through getPgPool() instead so
pools are tracked and properly cleaned up during database teardown.
@devin-ai-integration

Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@pyramation
pyramation merged commit f67b6f3 into main Mar 20, 2026
43 checks passed
@pyramation
pyramation deleted the devin/1774038000-fix-untracked-makepgservice-pools branch March 20, 2026 23:08
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