Skip to content

fix: lazy retry, handle cases, drop async-retry#1218

Merged
ferhatelmas merged 1 commit into
masterfrom
ferhat/lazy-retry
Jul 9, 2026
Merged

fix: lazy retry, handle cases, drop async-retry#1218
ferhatelmas merged 1 commit into
masterfrom
ferhat/lazy-retry

Conversation

@ferhatelmas

Copy link
Copy Markdown
Member

What kind of change does this PR introduce?

Refactor for performance

What is the current behavior?

Currently every transaction pays the price of async retry machinery. Machinery should be bolt on when it fails.

What is the new behavior?

Replace async-retry with a for loop, which is cheaper (no operation building, no options allocation, no promise, etc.), easier to maintain (no dependency), faster (no next tick).

Additional context

Start retrying connection state and broken client errors as well.
It's around 7x CPU, 6x GC friendly on success and 1.5x CPU, 3x GC friendly on retries.

@ferhatelmas ferhatelmas requested a review from a team as a code owner July 9, 2026 08:19
Copilot AI review requested due to automatic review settings July 9, 2026 08:19

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.

Pull request overview

This PR refactors Postgres transaction setup retries to remove the async-retry dependency and avoid retry machinery overhead on the common success path, while expanding the set of retryable transaction-setup failures.

Changes:

  • Replace async-retry with an inlined retry loop (bounded attempts + total time budget) for transaction setup in PgTenantConnection.
  • Add retry handling for additional transaction-setup failure modes (connection-state and broken-client/socket-level errors).
  • Remove async-retry and its type dependencies from package.json/package-lock.json, and add targeted unit tests for the new behavior.

Reviewed changes

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

File Description
src/internal/database/pg-connection.ts Replaces async-retry with a custom retry loop and adds retry classification for additional transient setup errors.
src/internal/database/pg-connection.test.ts Adds unit tests to validate the new retry behavior and the “no timers on success” performance goal.
package.json Drops async-retry and @types/async-retry dependencies.
package-lock.json Removes async-retry (and related types) from the lockfile.

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

Comment thread src/internal/database/pg-connection.ts
@coveralls

coveralls commented Jul 9, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 29005394843

Coverage increased (+0.02%) to 79.254%

Details

  • Coverage increased (+0.02%) from the base build.
  • Patch coverage: 1 uncovered change across 1 file (27 of 28 lines covered, 96.43%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
src/internal/database/pg-connection.ts 28 27 96.43%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 12581
Covered Lines: 10417
Line Coverage: 82.8%
Relevant Branches: 7293
Covered Branches: 5334
Branch Coverage: 73.14%
Branches in Coverage %: Yes
Coverage Strength: 423.31 hits per line

💛 - Coveralls

Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
@ferhatelmas ferhatelmas merged commit 1bfdcd7 into master Jul 9, 2026
26 checks passed
@ferhatelmas ferhatelmas deleted the ferhat/lazy-retry branch July 9, 2026 08:44
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.

4 participants