Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Vonng opened this issue Jun 30, 2024 · 6 comments
Open

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

Vonng opened this issue Jun 30, 2024 · 6 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@Vonng
Copy link
Owner

Vonng commented Jun 30, 2024

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 Vonng self-assigned this Jun 30, 2024
@Vonng Vonng added the bug Something isn't working label Jun 30, 2024
@Vonng Vonng added this to Pigsty Jun 30, 2024
@Vonng Vonng added this to the v0.7 milestone Jun 30, 2024
@Vonng
Copy link
Owner Author

Vonng commented Jun 30, 2024

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
Copy link
Collaborator

ringerc commented Jul 2, 2024

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.

@ringerc
Copy link
Collaborator

ringerc commented Sep 10, 2024

The revert commit was e80a810

@ringerc
Copy link
Collaborator

ringerc commented Sep 11, 2024

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

@Vonng
Copy link
Owner Author

Vonng commented Sep 13, 2024

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

@ringerc
Copy link
Collaborator

ringerc commented Sep 24, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: No status
Development

No branches or pull requests

2 participants