Skip to content

feat: configurable, reused DB connection pool - #425

Closed
jabbrwcky wants to merge 0 commit into
crossplane-contrib:masterfrom
jabbrwcky:feat/configurable-connection-pool
Closed

feat: configurable, reused DB connection pool#425
jabbrwcky wants to merge 0 commit into
crossplane-contrib:masterfrom
jabbrwcky:feat/configurable-connection-pool

Conversation

@jabbrwcky

Copy link
Copy Markdown
Contributor

Description of your changes

Every DB client previously opened a new *sql.DB via sql.Open on every query and closed it immediately, so the provider held an effectively unbounded number of short-lived connections against each server.

This PR introduces a process-wide, DSN-keyed connection-pool cache (pkg/clients/pool): managed resources targeting the same server (driver + DSN) now share a single bounded *sql.DB. Pools apply MaxOpenConns / MaxIdleConns / ConnMaxLifetime / ConnMaxIdleTime and are evicted after going idle, so a credential rotation (new DSN) does not leak pools.

The pool is tunable per ProviderConfig / ClusterProviderConfig via a new optional connectionPool block, for all three databases (MySQL, PostgreSQL, MSSQL), cluster- and namespaced-scoped. Omitted fields use defaults: maxOpenConnections=10, maxIdleConnections=5, maxConnLifetime=1h, maxConnIdleTime=10m.

Notable design points for reviewers:

  • The xsql.DB interface intentionally gains no Close(): the cache owns pool lifecycle, so the per-controller Disconnect methods stay no-ops.
  • Pools are shared per (driver, DSN); the config that first creates a pool for a DSN wins (in practice a DSN maps to one ProviderConfig).
  • Defaults are applied per-field and on a nil connectionPool, so existing ProviderConfigs keep working unchanged.

See docs/connection-pool.md for details.

Fixes #195

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable to ensure this PR is ready for review.

How has this code been tested

  • New unit tests for the pool cache (reuse per DSN, distinct pools per driver/DSN, idle-sweep eviction, active pools not swept).
  • Updated client and controller unit tests for the new constructor signatures.
  • make and make reviewable pass (generate is a no-op diff, lint and full go test ./... are green).

🤖 Generated with Claude Code

@jabbrwcky
jabbrwcky force-pushed the feat/configurable-connection-pool branch 3 times, most recently from b546801 to b072699 Compare July 23, 2026 12:55
@jabbrwcky jabbrwcky closed this Aug 18, 2026
@jabbrwcky
jabbrwcky force-pushed the feat/configurable-connection-pool branch from b072699 to fe39828 Compare August 18, 2026 12:05
@jabbrwcky

Copy link
Copy Markdown
Contributor Author

Superseded by #434 (same branch got wiped by a bad rebase, GitHub wouldn't let this PR reopen after recovery).

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.

Provider should use connection pooling or single transaction to minimize load on database when reconciling

1 participant