Skip to content

fix(pools): recover failed pool connections#75

Merged
abnegate merged 3 commits into
mainfrom
fix/pools-recover-failed-connections
Jul 17, 2026
Merged

fix(pools): recover failed pool connections#75
abnegate merged 3 commits into
mainfrom
fix/pools-recover-failed-connections

Conversation

@abnegate

Copy link
Copy Markdown
Member

Summary

Port of utopia-php/pools#33 to the canonical monorepo (the standalone pools repo is now a mirror with a redirect workflow, so the fix must land here to ship).

Root fix for the DAT-1904 production incident (a by-id read on nyc3 prod returned a different database's document) and the same defect family as the nyc3 write outage (utopia-php/database#895/#896, transaction-counter desync):

  • Pool::use() tracks whether the borrow callback threw and calls release($connection, failed: true) in its finally.
  • release(failed) runs recover(): if the resource exposes reset()/reconnect() it is recovered then reclaimed (for the DB adapters this swaps in a fresh PDO — discarding any wire-desynced socket / stale result frame — and zeroes the leaked inTransaction counter); if recovery is unavailable, returns false, or throws, the connection is destroyed instead of reclaimed.
  • Net invariant: a poisoned connection is never handed to the next borrower.

Also includes:

  • The recover-and-reuse happy-path regression test (a resource with a working reconnect() is reused, not destroyed).
  • Destroy-branch regression tests (recovery fails / returns false / no hooks / native resource / replacement fails).
  • utopia-php/telemetry lock bump 0.4.0 → 0.4.5 (within ^0.4): the observable-gauge tests read $gauge->callbacks[] (multi-observer, 0.4.1+); the old lock made testPoolTelemetry/testMultiplePoolsShareGauges… fail.
  • Monorepo canonical style applied (pint).

Testing

Full pools suite green locally: 49 tests, 168 assertions (Stack adapter; Swoole adapter shares the scopes).

Ship path

Merge here → release → cloud consumes via composer update (pools resolves through server-ce) → DAT-1904 guarded fleet-wide. The cloud-side containment (appwrite-labs/cloud#4827 Store identity guard) stays as defense-in-depth.

Supersedes utopia-php/pools#33.

🤖 Generated with Claude Code

Pool::use() now tracks borrow failure and release(failed) recovers the
connection via its reset()/reconnect() hooks before reclaiming, or destroys
it when recovery is unavailable or fails. A wire-desynced or transaction-
poisoned connection can no longer be handed to the next borrower — the root
of the DAT-1904 cross-document read bleed and the db#895/#896 write-outage
class. Includes the recover-and-reuse happy-path test and a telemetry lock
bump to 0.4.5 (observable-gauge multi-observer, required by the gauge tests).

Ported from utopia-php/pools#33 (standalone repo is now a mirror).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 17, 2026 07:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@greptile-apps

greptile-apps Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR updates pool cleanup so failed borrows do not return poisoned connections. The main changes are:

  • Recover or destroy failed pool connections before reuse.
  • Add last-resort cleanup when destroy handling fails.
  • Release grouped pool connections explicitly in reverse order.
  • Add tests for recovery, destroy, replacement failure, and grouped cleanup paths.
  • Update Composer metadata and the telemetry lock version used by the tests.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
packages/pools/src/Pools/Pool.php Adds failed-borrow release handling, recovery hooks, destroy fallback, and last-resort pool bookkeeping cleanup.
packages/pools/src/Pools/Group.php Reworks grouped borrowing to pop all requested connections and release every acquired connection during cleanup.
packages/pools/composer.json Corrects Composer suggestion metadata and keeps optional extension hints in the package manifest.
packages/pools/composer.lock Bumps the locked telemetry package version within the existing constraint.
packages/pools/tests/Pools/Scopes/PoolTestScope.php Adds tests for failed borrow recovery, destroy fallback, native resources, and replacement failure behavior.
packages/pools/tests/Pools/Scopes/GroupTestScope.php Adds tests for grouped cleanup after missing pools, telemetry recording, and release failures.

Reviews (3): Last reviewed commit: "style(pools): make test anonymous class ..." | Re-trigger Greptile

Comment thread packages/pools/src/Pools/Pool.php
abnegate and others added 2 commits July 17, 2026 21:43
…oy cleanup fails

When recovery fails and destroy() itself throws (e.g. the adapter's
synchronized lock fails before the active entry is removed), release()
previously swallowed the error with the connection still tracked in
$active and counted in connectionsCreated — permanently leaking a pool
slot. forget() now untracks the connection as a last resort, taking the
lock when possible and falling back to direct cleanup when the lock is
the thing that failed. Also drops two always-true assertIsResource
assertions and applies rector rules (readonly anonymous classes, never
returns) flagged by the monorepo checks.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The pools CI job fails at bin/monorepo check on Rector's dry-run:
ReadOnlyAnonymousClassRector requires the anonymous Stringable test
resource to be declared readonly since its only property is never
mutated after construction.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@abnegate
abnegate merged commit afd5df9 into main Jul 17, 2026
11 checks passed
@abnegate
abnegate deleted the fix/pools-recover-failed-connections branch July 17, 2026 10:19
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.

2 participants