Skip to content

fix: rebalance pool in place#1125

Merged
fenos merged 4 commits into
masterfrom
ferhat/knex-in-place-rebalance
May 28, 2026
Merged

fix: rebalance pool in place#1125
fenos merged 4 commits into
masterfrom
ferhat/knex-in-place-rebalance

Conversation

@ferhatelmas
Copy link
Copy Markdown
Member

@ferhatelmas ferhatelmas commented May 21, 2026

Fixing Two issues in tenant pool lifecycle:

  1. Cluster rebalance destroyed and recreated every cached pool, causing GC spikes.
  2. When a tenant's DB URL or credentials changed, the cached pool kept its stale connections — updateAgeOnGet prevented TTL eviction, so requests kept
    failing against a broken pool.

Changes

  • Cluster scale → in-place. TenantPool.rebalance mutates tarnPool.max and nudges _tryAcquireOrCreate(). No destroy, no GC churn.
  • onTenantConfigChange picks the right teardown:
    • databaseUrl / databasePoolUrl changed → hard destroy. Next request rebuilds against the new endpoint.
    • maxConnections changed → graceful recycle. New pool swapped into the cache synchronously; old pool drains in the background.
    • Pool-mode flip → hard destroy (existing).
  • New PoolManager.recycle(tenantId, settings) — synchronous cache swap, background drain via TenantPool.destroy(). Uses manuallyDestroyedPools
    to keep the cache dispose hook from racing the explicit destroy.

Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
@ferhatelmas ferhatelmas requested a review from a team as a code owner May 21, 2026 15:12
Copilot AI review requested due to automatic review settings May 21, 2026 15:12
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR changes tenant DB pool rebalancing to update existing Knex/Tarn pools in place (instead of draining/destroying and recreating), to avoid cache evictions and reduce GC churn during cluster autoscaling or tenant max-connection updates.

Changes:

  • Update TenantPool.rebalance() to modify the underlying Tarn pool’s max value in place.
  • Trigger Tarn’s acquire/create loop after resizing (when available) to apply the new max.
  • Update the unit test to assert the pool/Knex instance is preserved across rebalance and that destroy() is not called.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/internal/database/pool.ts Switches rebalance behavior from “replace/drain pool” to “resize Tarn pool max in place”.
src/internal/database/pool.test.ts Adjusts expectations to validate in-place resizing and no pool destruction during rebalance.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/internal/database/pool.ts
@coveralls
Copy link
Copy Markdown

coveralls commented May 21, 2026

Coverage Report for CI Build 26514061357

Coverage increased (+0.1%) to 75.265%

Details

  • Coverage increased (+0.1%) from the base build.
  • Patch coverage: 2 uncovered changes across 1 file (24 of 26 lines covered, 92.31%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
src/internal/database/tenant.ts 8 6 75.0%
Total (2 files) 26 24 92.31%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 10654
Covered Lines: 8449
Line Coverage: 79.3%
Relevant Branches: 6241
Covered Branches: 4267
Branch Coverage: 68.37%
Branches in Coverage %: Yes
Coverage Strength: 365.15 hits per line

💛 - Coveralls

@fenos fenos force-pushed the ferhat/knex-in-place-rebalance branch from 544eb1a to cd2014c Compare May 27, 2026 13:14
@fenos fenos force-pushed the ferhat/knex-in-place-rebalance branch from cd2014c to 6800125 Compare May 27, 2026 13:26
@fenos fenos merged commit 933acc1 into master May 28, 2026
21 checks passed
@fenos fenos deleted the ferhat/knex-in-place-rebalance branch May 28, 2026 05:55
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.

5 participants