search

Vonng / pg_exporter

Advanced PostgreSQL & Pgbouncer Metrics Exporter for Prometheus
https://pigsty.io
Apache License 2.0
163 stars 42 forks source link

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

Open Vonng opened 1 week ago

Vonng commented 1 week ago

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 commented 1 week ago

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. 😊

ringerc commented 5 days ago

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:

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.