fix(whatsmeow): reuse pooled authDB connection in StartClient (fixes #175) - #178
Open
wilsonborba wants to merge 1 commit into
Open
Conversation
StartClient called sqlstore.New(ctx, "postgres", config.PostgresAuthDB, ...) on every invocation, opening a brand new *sql.DB connection pool each time. The container (and its pool) was never closed, so every reconnect cycle leaked idle Postgres connections into the auth database. StartClient is re-entered on ReconnectClient, on the kill-channel branch of its own select loop, and on every QR pairing attempt that ends in a restart, so this leak accumulates quickly under normal instance create/reconnect activity and can exhaust max_connections. Reuse the already-pooled, bounded authDB *sql.DB (set up once in config.CreateAuthDB with SetMaxOpenConns/SetMaxIdleConns) via sqlstore.NewWithDB instead of opening a new pool per call. Falls back to the previous sqlstore.New behavior if authDB is unexpectedly nil, so this never panics on a nil *sql.DB. Verified locally: with the pool reused, 15 consecutive /instance/reconnect calls kept the Postgres connection count flat instead of growing past the configured pool cap. Fixes evolution-foundation#175
There was a problem hiding this comment.
Sorry @wilsonborba, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
Reviewer's GuideThis PR changes how the WhatsMeow service initializes its SQL container in StartClient so that it reuses the existing pooled Postgres authDB connection when available, ensuring upgrades are run explicitly and keeping logging behavior consistent across Postgres and SQLite paths while preserving a safe fallback to the previous behavior. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
StartClientcallssqlstore.New(ctx, "postgres", config.PostgresAuthDB, ...)on every invocation, opening a brand new*sql.DBconnection pool each time.container.Close()is never called, so the old pool (and every connection it opened) is simply dropped.StartClientis re-entered on:ReconnectClient, which callsStartInstance->StartClientagainStartClient's own select loop, which unconditionally restarts on any kill signalEach path leaks one unbounded connection pool. Reproduced locally against a real Postgres instance: 15 consecutive
POST /instance/reconnectcalls grewevogo_authconnections from a baseline of 2 to 32, exceeding the app's ownSetMaxOpenConns(25)cap, which confirms the leaked connections come from a pool outside the one the app manages. In production this exhaustsmax_connectionsunder normal instance create/reconnect activity, surfacing aspq: sorry, too many clients alreadyand QR pairing timeouts.Closes #175.
Fix
Reuse the already-pooled, bounded
authDB *sql.DB(whatsmeowServicealready holds it as a field, configured once inconfig.CreateAuthDBwithSetMaxOpenConns/SetMaxIdleConns/SetConnMaxLifetime/SetConnMaxIdleTime) viasqlstore.NewWithDB(w.authDB, "postgres", dbLog)instead of opening a new pool per call, then runcontainer.Upgrade(ctx)explicitly sinceNewWithDBdoes not auto-upgrade likesqlstore.Newdoes.Falls back to the previous
sqlstore.Newbehavior ifauthDBis unexpectedlynil(it should always be set whenPostgresAuthDBis configured, seemain.go), so this can't introduce a nil-pointer panic.No changes to the SQLite fallback path.
Verification
Tested against a real Postgres 192.168.x LAN instance with an actual WhatsApp session paired via QR:
/instance/reconnectConnected: true,LoggedIn: trueConnected: true,LoggedIn: truego build ./...passes on top ofdevelopwith thewhatsmeow-libsubmodule initialized.Notes
This is the same root cause already reported/attempted in #102, #117, #131, #168 and #174. This PR takes the smaller-diff approach from #168/#174 (reuse
w.authDBviaNewWithDBrather than introducing a new package-level singleton container), and additionally guards against a nilauthDBper the review feedback left on #174.Summary by Sourcery
Prevent WhatsApp client restarts from leaking PostgreSQL connection pools by reusing the configured authentication database connection.
Bug Fixes:
Enhancements: