Re: Psycopg 2.2.x issue with PgBouncer

On 8/2/10, Daniele Varrazzo <daniele.varrazzo@...> wrote:
> On Wed, Jul 28, 2010 at 4:28 PM, Marko Kreen <markokr@...> wrote:
>
>  > This behavior is bad.  The original user code was buggy - it closed
>  > the connection while in mid-transaction.  PgBouncer was correct
>  > to drop the connection to force the server to do rollback.
>  >
>  > I recently had use-case in production env where such behaviour
>  > (making buggy client act as non-buggy client) was clearly broken:
>  >
>  > There was COPY-to-COPY pipeline, where target side got error.
>  > Now if I do .close() on source side, what happens?  It starts
>  > to consume data until command end... Which is silly.
>  >
>  > Same can happen with big (async?) SELECT, where it's impossible
>  > to get rid of the connection.  Even __del__ will consume data...
>  >
>  > Only way to get out of such situation would be:
>  >
>  >  os.close(db.cursor().fileno())
>  >
>  > but it seems silly to force users to use such workaround.
>  >
>  > My conclusion: .close() and .__del__() should close the
>  > connection immediately, without any workarounds, to guarantee
>  > predictable behavior.  There is no need to drain data to
>  > issue ROLLBACK, because mid-trancation disconnect will
>  > do that anyway.
>
>
> For me doing the least possible on __del__, mostly don't try to issue
>  queries, is a sensible approach.

Yes.

>  OTOH, the result of the bug was exactly what Marko suggests: issuing
>  PQfinish without a ROLLBACK, which has been reported not playing well
>  with PgBouncer.

For pgbouncer to "play well" with such behaviour, it would need
to have quite a lot more complex protocol state-machine.

Remember, the client connection can go down at any point during
packet exchange.  When can the connection be recovered and when not?

>  So, is this the tradeoff we should consider? trying to ROLLBACK on
>  close() makes us try to do a potential big amount of work in the wrong
>  moment; closing brutally makes pooling impossible.

Well, technically you could issue ROLLBACK if you are sure the
connection is <idle in transaction> - that there are no ongoing
queries going on.  This avoids the unnecessary draining problem.

But even that can suck - lets say I have 2 connections open, both
in transaction. I issue a query on one, but network goes down.
The query gets network timeout, now i close both connections.
(Or __del__ ...)  Why do I need to wait for network timeout on other
connection too?

>  I don't know PgBouncer at all so I'm asking Jason -- or whoever can
>  answer: is just the badly-closed connection to be forced out of the
>  pool or is the "unclean server" is serious enough to make it consider
>  the entire pool inconsistent?

Only one connection would be dropped.

>  On the base that you can always call rollback() before closing/gc-ing
>  the connection, I'm actually tempted to make the buggy behaviour "a
>  feature".

You seem to forget that it can also call .commit().  And in
autocommit mode the issue should not even arise, but I'm
not sure about current code...

Anyway, there is only one sort-of-valid excuse for dropping .rollback:
to avoid network roundtrip, as postgres does it on close anyway.

But that plays badly with pooling, as you noticed.

And when db driver does it anyway, the excuse is void.

IOW, doing anything else then PQfinish in .close() [not even speaking
about .__del__()] is wrong.

--

-- 
marko