pgbouncer appears to lose sync when parameter is set in transaction mode #52

Closed
roji opened this Issue May 24, 2015 · 7 comments

Comments

Projects
None yet
3 participants
@roji

roji commented May 24, 2015

I'm working on the .NET driver for PostgreSQL, Npgsql, and I think I've come across an issue.

During my tests, I set pgbouncer in transaction mode, and sent a packet containing a SET statement_timeout=30000 simple query message, followed by a regular extended protocol query: this seemed to make pgbouncer "lose sync" in some way. I know SET isn't supported in transaction mode, but I'd have expected a clear error rather than the strange behavior below:

Messages sent (as in wireshark): Q/P/D/B/E/S (the first Q is the SET)
Messages received: C/Z

pgbouncer logs:

2015-05-24 11:58:13.192 24481 WARNING S-0x8a677a0: npgsql_tests/npgsql_tests@127.0.0.1:5432 got packet 'S' from server when not linked
2015-05-24 11:58:13.192 24481 WARNING S-0x8a677a0: npgsql_tests/npgsql_tests@127.0.0.1:5432 got packet 'S' from server when not linked
2015-05-24 11:58:13.192 24481 WARNING S-0x8a677a0: npgsql_tests/npgsql_tests@127.0.0.1:5432 got packet 'C' from server when not linked
2015-05-24 11:58:13.192 24481 WARNING S-0x8a677a0: npgsql_tests/npgsql_tests@127.0.0.1:5432 got packet 'Z' from server when not linked

pgbouncer 1.5.4 on Ubuntu utopic, PostgreSQL 9.3. Let me know if there's any other info that can help.

@markokr

This comment has been minimized.

Show comment
Hide comment
@markokr

markokr Jul 31, 2015

Contributor

Losing the setting by getting differerent server connection should be normal. But not losing protocol sync.

Could you send pgbouncer -v -v -v log about that query exchange?

Contributor

markokr commented Jul 31, 2015

Losing the setting by getting differerent server connection should be normal. But not losing protocol sync.

Could you send pgbouncer -v -v -v log about that query exchange?

@markokr markokr added the moreinfo label Aug 3, 2015

@roji

This comment has been minimized.

Show comment
Hide comment
@roji

roji Aug 3, 2015

Here you go: https://gist.github.com/roji/4f3205c2e8ffcdd786fb

My repro simply connects, sends a Q with set statement_timeout=30000, sends a trivial P/D/B/E/S sequence and closes the connection. Then it tries to connect again and sends another trivial P/D/B/E/S and the error occurs.

I can reproduce this really easily here, let me know if you need any more info.

roji commented Aug 3, 2015

Here you go: https://gist.github.com/roji/4f3205c2e8ffcdd786fb

My repro simply connects, sends a Q with set statement_timeout=30000, sends a trivial P/D/B/E/S sequence and closes the connection. Then it tries to connect again and sends another trivial P/D/B/E/S and the error occurs.

I can reproduce this really easily here, let me know if you need any more info.

roji added a commit to roji/Npgsql that referenced this issue Aug 3, 2015

@markokr markokr added bug and removed moreinfo labels Aug 6, 2015

@markokr

This comment has been minimized.

Show comment
Hide comment
@markokr

markokr Aug 6, 2015

Contributor

problem seems to be that client sent stream of several sync commands (Q and S) to server and pgbouncer switches server after the first.

need to think how to solve it. keep a counter?

Contributor

markokr commented Aug 6, 2015

problem seems to be that client sent stream of several sync commands (Q and S) to server and pgbouncer switches server after the first.

need to think how to solve it. keep a counter?

@markokr

This comment has been minimized.

Show comment
Hide comment
@markokr

markokr Aug 6, 2015

Contributor

Counter should solve bind-sync-exec problem in #44 too?

Contributor

markokr commented Aug 6, 2015

Counter should solve bind-sync-exec problem in #44 too?

@PJMODOS

This comment has been minimized.

Show comment
Hide comment
@PJMODOS

PJMODOS Aug 6, 2015

Contributor

yeah I think counter would help with #44 as well

Contributor

PJMODOS commented Aug 6, 2015

yeah I think counter would help with #44 as well

markokr added a commit to markokr/pgbouncer-dev that referenced this issue Aug 8, 2015

Support pipelining - count expected ReadyForQuery packets.
This avoids releasing server too early.

Should fix #44 and #52.
@markokr

This comment has been minimized.

Show comment
Hide comment
@markokr

markokr Aug 8, 2015

Contributor

Pushed experimental patch to master, please test if it solves the issue.

Contributor

markokr commented Aug 8, 2015

Pushed experimental patch to master, please test if it solves the issue.

@markokr markokr added the fixed label Aug 8, 2015

@roji

This comment has been minimized.

Show comment
Hide comment
@roji

roji Aug 19, 2015

Confirmed, using the latest master the problem doesn't occur, thanks!

roji commented Aug 19, 2015

Confirmed, using the latest master the problem doesn't occur, thanks!

@roji roji closed this Aug 19, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment