diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8a3a1811d..f6827de32 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -134,6 +134,33 @@ jobs: PGX_SSL_PASSWORD: ${{ matrix.pgx-ssl-password }} PGX_TEST_TLS_CLIENT_CONN_STRING: ${{ matrix.pgx-test-tls-client-conn-string }} + test-pgbouncer: + name: Test PgBouncer with PostgreSQL 18 + runs-on: ubuntu-22.04 + + steps: + - name: Check out code into the Go module directory + uses: actions/checkout@v5 + + - name: Set up Go 1.26 + uses: actions/setup-go@v6 + with: + go-version: "1.26" + + - name: Set up PostgreSQL 18 + run: ci/setup_test.bash + env: + PGVERSION: "18" + + - name: Set up PgBouncer + run: ci/setup_pgbouncer.bash + + - name: Test PgBouncer + run: go test -parallel=1 -race -count=1 -run 'TestPgbouncer|TestConnContextCanceledCancelsRunningQueryOnServer' ./... + env: + PGX_TEST_DATABASE: "host=127.0.0.1 user=pgx_md5 password=secret dbname=pgx_test" + PGX_TEST_PGBOUNCER_CONN_STRING: "host=127.0.0.1 port=6432 user=pgx_md5 password=secret dbname=pgx_test" + test-windows: name: Test Windows runs-on: windows-latest diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2283ae670..9a8d1b020 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -130,6 +130,9 @@ psql --no-psqlrc -f testsetup/postgresql_setup.sql ### PgBouncer There are tests specific for PgBouncer that will be executed if `PGX_TEST_PGBOUNCER_CONN_STRING` is set. +The test PgBouncer must be version 1.21.0 or newer, use transaction pooling, and have `max_prepared_statements` set to a +non-zero value. This ensures the tests cover PgBouncer's protocol-level named prepared statement support in addition to +the pgx query modes that do not use named prepared statements. ### Optional Tests diff --git a/ci/setup_pgbouncer.bash b/ci/setup_pgbouncer.bash new file mode 100755 index 000000000..18271aa13 --- /dev/null +++ b/ci/setup_pgbouncer.bash @@ -0,0 +1,20 @@ +#!/usr/bin/env bash +set -euxo pipefail + +sudo apt-get install -y pgbouncer +sudo systemctl stop pgbouncer + +sudo install -o postgres -g postgres -m 0750 -d /etc/pgbouncer +sudo install -o postgres -g postgres -m 0640 testsetup/pgbouncer.ini /etc/pgbouncer/pgbouncer.ini +sudo install -o postgres -g postgres -m 0640 testsetup/pgbouncer-userlist.txt /etc/pgbouncer/userlist.txt +sudo -u postgres pgbouncer -d /etc/pgbouncer/pgbouncer.ini + +for _ in {1..30}; do + if PGPASSWORD=secret psql "host=127.0.0.1 port=6432 user=pgx_md5 dbname=pgx_test" -c "select 1"; then + exit 0 + fi + sleep 1 +done + +sudo cat /var/log/postgresql/pgbouncer.log || true +exit 1 diff --git a/conn.go b/conn.go index 1f31c40fc..7067bfd45 100644 --- a/conn.go +++ b/conn.go @@ -36,9 +36,10 @@ type ConnConfig struct { DescriptionCacheCapacity int // DefaultQueryExecMode controls the default mode for executing queries. By default pgx uses the extended protocol - // and automatically prepares and caches prepared statements. However, this may be incompatible with proxies such as - // PGBouncer. In this case it may be preferable to use [QueryExecModeExec] or [QueryExecModeSimpleProtocol]. The same - // functionality can be controlled on a per query basis by passing a [QueryExecMode] as the first query argument. + // and automatically prepares and caches prepared statements. This may be incompatible with proxies such as PgBouncer + // unless they are configured to support protocol-level prepared statements. In an incompatible configuration it may + // be preferable to use [QueryExecModeExec] or [QueryExecModeSimpleProtocol]. The same functionality can be controlled + // on a per query basis by passing a [QueryExecMode] as the first query argument. DefaultQueryExecMode QueryExecMode createdByParseConfig bool // Used to enforce created by ParseConfig rule. diff --git a/doc.go b/doc.go index 5e4870191..f20b3f68f 100644 --- a/doc.go +++ b/doc.go @@ -210,8 +210,18 @@ implemented on top of [pgconn.PgConn]. The [Conn.PgConn] method can be used to a PgBouncer -By default pgx automatically uses prepared statements. Prepared statements are incompatible with PgBouncer. This can be -disabled by setting a different [QueryExecMode] in [ConnConfig.DefaultQueryExecMode]. +By default pgx automatically uses protocol-level named prepared statements. PgBouncer 1.21.0 and newer can use these +prepared statements in transaction and statement pooling modes when its max_prepared_statements setting is greater than +zero. In this configuration the default [QueryExecModeCacheStatement] mode may be used. See the PgBouncer documentation +for limitations and configuration details: https://www.pgbouncer.org/config.html#max_prepared_statements. + +When using an older PgBouncer version or when prepared statement support is disabled, set +[ConnConfig.DefaultQueryExecMode] to [QueryExecModeExec]. [QueryExecModeCacheDescribe] and +[QueryExecModeSimpleProtocol] are also compatible with PgBouncer. Prefer [QueryExecModeExec] over +[QueryExecModeSimpleProtocol] whenever possible. Do not use [QueryExecModeDescribeExec] with transaction pooling because +PgBouncer may assign a different server connection between the describe and execute steps. + +SQL-level prepared statements created with PREPARE are not supported by PgBouncer in transaction pooling mode. */ package pgx diff --git a/pgbouncer_test.go b/pgbouncer_test.go index 13d07f34c..bbdb1d9bb 100644 --- a/pgbouncer_test.go +++ b/pgbouncer_test.go @@ -2,74 +2,169 @@ package pgx_test import ( "context" + "errors" + "fmt" "os" "testing" + "time" "github.com/jackc/pgx/v5" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/sync/errgroup" ) -func TestPgbouncerStatementCacheDescribe(t *testing.T) { +func TestPgbouncerQueryExecModes(t *testing.T) { connString := os.Getenv("PGX_TEST_PGBOUNCER_CONN_STRING") if connString == "" { t.Skipf("Skipping due to missing environment variable %v", "PGX_TEST_PGBOUNCER_CONN_STRING") } - config := mustParseConfig(t, connString) - config.DefaultQueryExecMode = pgx.QueryExecModeCacheDescribe - config.DescriptionCacheCapacity = 1024 + tests := []struct { + name string + mode pgx.QueryExecMode + statementCacheCapacity int + descriptionCacheCapacity int + }{ + { + name: "cache statement", + mode: pgx.QueryExecModeCacheStatement, + statementCacheCapacity: 32, + }, + { + name: "cache describe", + mode: pgx.QueryExecModeCacheDescribe, + descriptionCacheCapacity: 32, + }, + { + name: "exec", + mode: pgx.QueryExecModeExec, + }, + { + name: "simple protocol", + mode: pgx.QueryExecModeSimpleProtocol, + }, + } - testPgbouncer(t, config, 10, 100) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + config := mustParseConfig(t, connString) + config.DefaultQueryExecMode = tt.mode + config.StatementCacheCapacity = tt.statementCacheCapacity + config.DescriptionCacheCapacity = tt.descriptionCacheCapacity + + testPgbouncer(t, config, 10, 100) + }) + } } -func TestPgbouncerSimpleProtocol(t *testing.T) { +func TestPgbouncerStatementCacheEviction(t *testing.T) { connString := os.Getenv("PGX_TEST_PGBOUNCER_CONN_STRING") if connString == "" { t.Skipf("Skipping due to missing environment variable %v", "PGX_TEST_PGBOUNCER_CONN_STRING") } config := mustParseConfig(t, connString) - config.DefaultQueryExecMode = pgx.QueryExecModeSimpleProtocol + config.DefaultQueryExecMode = pgx.QueryExecModeCacheStatement + config.StatementCacheCapacity = 8 - testPgbouncer(t, config, 10, 100) + conn, err := pgx.ConnectConfig(t.Context(), config) + require.NoError(t, err) + defer conn.Close(t.Context()) + + for i := range 200 { + sql := fmt.Sprintf("select $1::int4 /* statement cache eviction %d */", i) + var n int32 + err := conn.QueryRow(t.Context(), sql, int32(i)).Scan(&n) + require.NoError(t, err) + require.EqualValues(t, i, n) + } } func testPgbouncer(t *testing.T, config *pgx.ConnConfig, workers, iterations int) { - doneChan := make(chan struct{}) + ctx, cancel := context.WithTimeout(t.Context(), 2*time.Minute) + defer cancel() + + eg, ctx := errgroup.WithContext(ctx) for range workers { - go func() { - defer func() { doneChan <- struct{}{} }() - conn, err := pgx.ConnectConfig(context.Background(), config) - require.Nil(t, err) - defer closeConn(t, conn) - - for range iterations { - var i32 int32 - var i64 int64 - var f32 float32 - var s string - var s2 string - err = conn.QueryRow(context.Background(), "select 1::int4, 2::int8, 3::float4, 'hi'::text").Scan(&i32, &i64, &f32, &s) - require.NoError(t, err) - assert.Equal(t, int32(1), i32) - assert.Equal(t, int64(2), i64) - assert.Equal(t, float32(3), f32) - assert.Equal(t, "hi", s) - - err = conn.QueryRow(context.Background(), "select 1::int8, 2::float4, 'bye'::text, 4::int4, 'whatever'::text").Scan(&i64, &f32, &s, &i32, &s2) - require.NoError(t, err) - assert.Equal(t, int64(1), i64) - assert.Equal(t, float32(2), f32) - assert.Equal(t, "bye", s) - assert.Equal(t, int32(4), i32) - assert.Equal(t, "whatever", s2) + eg.Go(func() (err error) { + conn, err := pgx.ConnectConfig(ctx, config) + if err != nil { + return err } - }() + defer func() { + err = errors.Join(err, conn.Close(ctx)) + }() + + return exercisePgbouncerConn(ctx, conn, iterations) + }) } - for range workers { - <-doneChan + require.NoError(t, eg.Wait()) +} + +func exercisePgbouncerConn(ctx context.Context, conn *pgx.Conn, iterations int) error { + for i := range iterations { + var i32 int32 + var i64 int64 + var f32 float32 + var s string + + err := conn.QueryRow(ctx, "select $1::int4, $2::int8, $3::float4, $4::text", int32(i), int64(i+1), float32(i+2), "hi").Scan(&i32, &i64, &f32, &s) + if err != nil { + return err + } + if i32 != int32(i) || i64 != int64(i+1) || f32 != float32(i+2) || s != "hi" { + return fmt.Errorf("unexpected query result: %d, %d, %f, %q", i32, i64, f32, s) + } + } + + commandTag, err := conn.Exec(ctx, "select $1::int4", int32(42)) + if err != nil { + return err } + if commandTag.String() != "SELECT 1" { + return fmt.Errorf("unexpected command tag: %s", commandTag) + } + + tx, err := conn.Begin(ctx) + if err != nil { + return err + } + var txValue int32 + if err := tx.QueryRow(ctx, "select $1::int4", int32(43)).Scan(&txValue); err != nil { + _ = tx.Rollback(ctx) + return err + } + if txValue != 43 { + _ = tx.Rollback(ctx) + return fmt.Errorf("unexpected transaction query result: %d", txValue) + } + if err := tx.Commit(ctx); err != nil { + return err + } + + batch := &pgx.Batch{} + batch.Queue("select $1::int4", int32(44)) + batch.Queue("select $1::text", "batch") + batchResults := conn.SendBatch(ctx, batch) + + var batchInt int32 + if err := batchResults.QueryRow().Scan(&batchInt); err != nil { + _ = batchResults.Close() + return err + } + var batchText string + if err := batchResults.QueryRow().Scan(&batchText); err != nil { + _ = batchResults.Close() + return err + } + if err := batchResults.Close(); err != nil { + return err + } + if batchInt != 44 || batchText != "batch" { + return fmt.Errorf("unexpected batch query results: %d, %q", batchInt, batchText) + } + + return nil } diff --git a/testsetup/pgbouncer-userlist.txt b/testsetup/pgbouncer-userlist.txt new file mode 100644 index 000000000..97092ec2c --- /dev/null +++ b/testsetup/pgbouncer-userlist.txt @@ -0,0 +1 @@ +"pgx_md5" "secret" diff --git a/testsetup/pgbouncer.ini b/testsetup/pgbouncer.ini new file mode 100644 index 000000000..30f0c3481 --- /dev/null +++ b/testsetup/pgbouncer.ini @@ -0,0 +1,22 @@ +[databases] +pgx_test = host=127.0.0.1 port=5432 dbname=pgx_test user=pgx_md5 password=secret + +[pgbouncer] +listen_addr = 127.0.0.1 +listen_port = 6432 +unix_socket_dir = /var/run/postgresql + +auth_type = md5 +auth_file = /etc/pgbouncer/userlist.txt +admin_users = pgx_md5 + +pool_mode = transaction +default_pool_size = 2 +max_client_conn = 100 +max_prepared_statements = 200 + +server_tls_sslmode = disable +client_tls_sslmode = disable + +logfile = /var/log/postgresql/pgbouncer.log +pidfile = /var/run/postgresql/pgbouncer.pid