pg_exporter icon indicating copy to clipboard operation
pg_exporter copied to clipboard

New `jackc/pgx` driver does not work well with pgbouncer metrics

Open Vonng opened this issue 1 year ago • 6 comments

Two problems with the new jackc/pgx drvier when working with pgBouncer:

Pgbouncer works with SimpleQueryProtocol, which can be solved by:

config, err := pgx.ParseConfig(s.dsn)
config.DefaultQueryExecMode = pgx.QueryExecModeSimpleProtocol

The driver will send -- ping command to pgbouncer, which will trigger an error log periodically

Jun 30 02:11:47 rocky8 pgbouncer[109508]: C-0x55e5664ad6c0: pgbouncer/dbuser_monitor@unix(5155):6432 pooler error: invalid command '-- ping', use SHOW HELP;
Jun 30 02:11:47 rocky8 pgbouncer[109508]: C-0x55e5664ad6c0: pgbouncer/dbuser_monitor@unix(5155):6432 pooler error: invalid command '-- ping', use SHOW HELP;
Jun 30 02:11:56 rocky8 pgbouncer[109508]: C-0x55e5664ad6c0: pgbouncer/dbuser_monitor@unix(5155):6432 pooler error: invalid command '-- ping', use SHOW HELP;

The latter one seems tricky; I'm looking into it.

;-) Any thoughts on this ? @ringerc

Vonng avatar Jun 30 '24 02:06 Vonng

I couldn't find a way to override the jackc/pgx driver's Ping behavior with pgbouncer.

So I revert PR 52 temporarily until there's a proper solution.

Besides, using lib/pq will reduce binary size by 5MB (20MB vs 15MB), which may be another advantage. 😊

Vonng avatar Jun 30 '24 02:06 Vonng

Sorry about that! Yes, thanks for reverting. I can look into it later and see. I can work around the issues I have with lib/pq for now or carry a local patch.

Logging an error for a query like -- ping query is a bug in pgbouncer IMO. PostgreSQL itself accepts empty query strings, and many clients use these for a keepalive / liveness check.

What pgbouncer version are you using though? It should support the extended query protocol fine, and in fact supports disabling the simple query protocol.

Some related discussion:

  • https://github.com/jackc/pgx/issues/272
  • https://github.com/jackc/pgx/issues/1738

I'd like to get it onto an updated driver at some point.

Quite surprised by the image size change, but the real world resource use at runtime is probably what matters.

At some point I hope to get time to write some test cases and a harness setup with Ginkgo or something, so this sort of thing can be checked for more effectively.

ringerc avatar Jul 02 '24 04:07 ringerc

The revert commit was https://github.com/Vonng/pg_exporter/commit/e80a8105a1800c47c4db42112678b5d39b70a66e

ringerc avatar Sep 10 '24 02:09 ringerc

@Vonng Can you confirm the pgbouncer version from the above?

Sounds like it's solve-able with a config option to force simple-query mode for old proxies that are incompatible with the v3 query protocol.

ringerc avatar Sep 11 '24 01:09 ringerc

pgbouncer 1.22 / 1.23, but I guess it applies to all versions.

Vonng avatar Sep 13 '24 04:09 Vonng

Thanks. Will try to chase this up. Currently slammed.

ringerc avatar Sep 24 '24 01:09 ringerc