From 546f255b4618d0c3b6aa9334ecfccfdd867fb7e1 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Thu, 9 Apr 2026 16:36:15 -0700 Subject: [PATCH] fix(filer/postgres): use pgx v5 API for PgBouncer simple protocol (#9010) * fix(filer/postgres): use pgx v5 API for PgBouncer simple protocol In pgx/v5 the `prefer_simple_protocol` DSN parameter was removed, so appending it to the connection string caused PgBouncer/PostgreSQL to reject it as an unknown startup parameter: FATAL: unsupported startup parameter: prefer_simple_protocol (SQLSTATE 08P01) Parse the DSN with pgx.ParseConfig and, when pgbouncer_compatible is set, configure DefaultQueryExecMode = QueryExecModeSimpleProtocol and disable the statement/description caches. Register the config via stdlib.RegisterConnConfig before sql.Open. Fixes #9005 * refactor(filer/postgres): extract shared OpenPGXDB helper with cleanup Extract the pgx v5 ParseConfig/RegisterConnConfig/sql.Open/Ping logic into a shared postgres.OpenPGXDB helper used by both postgres and postgres2 filer stores, eliminating ~60 lines of duplication. The helper also unregisters the conn config via stdlib.UnregisterConnConfig on every failure path (sql.Open error, Ping error) so we do not leak entries in stdlib's global connection config map when initialization fails. * refactor(filer/postgres): use stdlib.OpenDB to avoid conn config leak Switch OpenPGXDB from RegisterConnConfig + sql.Open("pgx", connStr) to stdlib.OpenDB(*connConfig). The former leaks an entry in stdlib's global conn config map on every successful initialization; stdlib.OpenDB takes the config directly and keeps no global registration. Addresses CodeRabbit review feedback on #9010. --- weed/filer/postgres/pgx_conn.go | 50 +++++++++++++++++++++++++ weed/filer/postgres/postgres_store.go | 30 ++------------- weed/filer/postgres2/postgres2_store.go | 29 ++------------ 3 files changed, 58 insertions(+), 51 deletions(-) create mode 100644 weed/filer/postgres/pgx_conn.go diff --git a/weed/filer/postgres/pgx_conn.go b/weed/filer/postgres/pgx_conn.go new file mode 100644 index 000000000..d53c3811d --- /dev/null +++ b/weed/filer/postgres/pgx_conn.go @@ -0,0 +1,50 @@ +package postgres + +import ( + "database/sql" + "fmt" + "time" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/stdlib" +) + +// OpenPGXDB parses the given DSN into a pgx ConnConfig, applies PgBouncer +// compatibility settings when requested, opens a *sql.DB via +// stdlib.OpenDB, and verifies it with Ping. +// +// In pgx/v5 the prefer_simple_protocol DSN parameter was removed, so simple +// protocol mode must be configured on the ConnConfig via +// DefaultQueryExecMode. We use stdlib.OpenDB(config) rather than +// RegisterConnConfig + sql.Open so we don't leak entries in stdlib's global +// connection config map on either success or failure paths. +// +// adaptedSqlUrl is used only for error messages (the caller is expected to +// have redacted any password). +func OpenPGXDB(sqlUrl, adaptedSqlUrl string, pgbouncerCompatible bool, maxIdle, maxOpen, maxLifetimeSeconds int) (*sql.DB, error) { + connConfig, parseErr := pgx.ParseConfig(sqlUrl) + if parseErr != nil { + return nil, fmt.Errorf("can not parse connection config for %s error:%v", adaptedSqlUrl, parseErr) + } + + // PgBouncer compatibility: use the simple query protocol and disable + // statement caching. This avoids prepared statement issues with + // PgBouncer's transaction pooling mode. + if pgbouncerCompatible { + connConfig.DefaultQueryExecMode = pgx.QueryExecModeSimpleProtocol + connConfig.StatementCacheCapacity = 0 + connConfig.DescriptionCacheCapacity = 0 + } + + db := stdlib.OpenDB(*connConfig) + db.SetMaxIdleConns(maxIdle) + db.SetMaxOpenConns(maxOpen) + db.SetConnMaxLifetime(time.Duration(maxLifetimeSeconds) * time.Second) + + if err := db.Ping(); err != nil { + db.Close() + return nil, fmt.Errorf("connect to %s error:%v", adaptedSqlUrl, err) + } + + return db, nil +} diff --git a/weed/filer/postgres/postgres_store.go b/weed/filer/postgres/postgres_store.go index 1c60575ee..6778bc59e 100644 --- a/weed/filer/postgres/postgres_store.go +++ b/weed/filer/postgres/postgres_store.go @@ -8,12 +8,8 @@ package postgres import ( - "database/sql" - "fmt" "strconv" - "time" - _ "github.com/jackc/pgx/v5/stdlib" "github.com/seaweedfs/seaweedfs/weed/filer" "github.com/seaweedfs/seaweedfs/weed/filer/abstract_sql" "github.com/seaweedfs/seaweedfs/weed/util" @@ -68,12 +64,6 @@ func (store *PostgresStore) initialize(upsertQuery string, enableUpsert bool, us // pgx-optimized connection string with better timeouts and connection handling sqlUrl := "connect_timeout=30" - // PgBouncer compatibility: add prefer_simple_protocol=true when needed - // This avoids prepared statement issues with PgBouncer's transaction pooling mode - if pgbouncerCompatible { - sqlUrl += " prefer_simple_protocol=true" - } - if hostname != "" { sqlUrl += " host=" + hostname } @@ -113,23 +103,11 @@ func (store *PostgresStore) initialize(upsertQuery string, enableUpsert bool, us sqlUrl += " search_path=" + schema adaptedSqlUrl += " search_path=" + schema } - var dbErr error - store.DB, dbErr = sql.Open("pgx", sqlUrl) - if dbErr != nil { - if store.DB != nil { - store.DB.Close() - } - store.DB = nil - return fmt.Errorf("can not connect to %s error:%v", adaptedSqlUrl, dbErr) - } - - store.DB.SetMaxIdleConns(maxIdle) - store.DB.SetMaxOpenConns(maxOpen) - store.DB.SetConnMaxLifetime(time.Duration(maxLifetimeSeconds) * time.Second) - - if err = store.DB.Ping(); err != nil { - return fmt.Errorf("connect to %s error:%v", adaptedSqlUrl, err) + db, openErr := OpenPGXDB(sqlUrl, adaptedSqlUrl, pgbouncerCompatible, maxIdle, maxOpen, maxLifetimeSeconds) + if openErr != nil { + return openErr } + store.DB = db return nil } diff --git a/weed/filer/postgres2/postgres2_store.go b/weed/filer/postgres2/postgres2_store.go index 23d811816..d563ec322 100644 --- a/weed/filer/postgres2/postgres2_store.go +++ b/weed/filer/postgres2/postgres2_store.go @@ -9,12 +9,9 @@ package postgres2 import ( "context" - "database/sql" "fmt" "strconv" - "time" - _ "github.com/jackc/pgx/v5/stdlib" "github.com/seaweedfs/seaweedfs/weed/filer" "github.com/seaweedfs/seaweedfs/weed/filer/abstract_sql" "github.com/seaweedfs/seaweedfs/weed/filer/postgres" @@ -73,12 +70,6 @@ func (store *PostgresStore2) initialize(createTable, upsertQuery string, enableU // pgx-optimized connection string with better timeouts and connection handling sqlUrl := "connect_timeout=30" - // PgBouncer compatibility: add prefer_simple_protocol=true when needed - // This avoids prepared statement issues with PgBouncer's transaction pooling mode - if pgbouncerCompatible { - sqlUrl += " prefer_simple_protocol=true" - } - if hostname != "" { sqlUrl += " host=" + hostname } @@ -118,23 +109,11 @@ func (store *PostgresStore2) initialize(createTable, upsertQuery string, enableU sqlUrl += " search_path=" + schema adaptedSqlUrl += " search_path=" + schema } - var dbErr error - store.DB, dbErr = sql.Open("pgx", sqlUrl) - if dbErr != nil { - if store.DB != nil { - store.DB.Close() - } - store.DB = nil - return fmt.Errorf("can not connect to %s error:%v", adaptedSqlUrl, dbErr) - } - - store.DB.SetMaxIdleConns(maxIdle) - store.DB.SetMaxOpenConns(maxOpen) - store.DB.SetConnMaxLifetime(time.Duration(maxLifetimeSeconds) * time.Second) - - if err = store.DB.Ping(); err != nil { - return fmt.Errorf("connect to %s error:%v", adaptedSqlUrl, err) + db, openErr := postgres.OpenPGXDB(sqlUrl, adaptedSqlUrl, pgbouncerCompatible, maxIdle, maxOpen, maxLifetimeSeconds) + if openErr != nil { + return openErr } + store.DB = db if err = store.CreateTable(context.Background(), abstract_sql.DEFAULT_TABLE); err != nil { return fmt.Errorf("init table %s: %v", abstract_sql.DEFAULT_TABLE, err)