Close connection when check query errors #683
Conversation
There was a problem hiding this comment.
I'm not entirely sure what issue this is fixing, could you describe the scenario a bit more clearly? I haven't used CockroachDB a lot, so I likely just need a bit more background to understand.
TBH, I'm not even sure I really understand the purpose of the server_check_query at all. From my look at the code of server_check_query, it's run after a previous query ends to check if the server is still working. I guess this is mostly to find out quickly if the connection got completely broken during the previous query, but even if it succeeds, it's still very possible that the connection breaks inbetween the server_check_query and the next query that's sent over the connection.
Also regarding this:
Additionally, newly logged in connections are now checked when check delay is set to zero.
What is this necessary for?
Currently, pgbouncer will ignore errors returned by the check query. This is mostly ok for traditional Postgres, but creates issues when using pgbouncer in front of high availability pg compatible databases (ex: CockroachDB). Essentially, the pooler will hand out broken connections if the database is draining. This change simply closes SV_TESTED connections that receive errors, allowing for seamless failover. Additionally, newly logged in connections are now checked when check delay is set to zero.