Python database interface to postgresql development list

Marko Kreen | 2 Aug 2010 09:21

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

(Continue reading)

headers

Daniele Varrazzo | 2 Aug 2010 13:45

Re: Psycopg 2.2.x issue with PgBouncer

On Mon, Aug 2, 2010 at 8:21 AM, Marko Kreen <markokr@...> wrote:
> On 8/2/10, Daniele Varrazzo <daniele.varrazzo@...> wrote:

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

No, I just mentioned rollback because this would replicate, from a
data state PoV, what happens to the server both if the client closes
the connection with a straight PQfinish and what used to happen in
psycopg on close()/GC before introducing the bug. commit() would
obviously close the transaction too but replacing close() with
rollback() (or prepending a rollback() call) would keep the semantic
of existing programs untouched, while giving PgBouncer the correct
message.

If this is agreed, I can change the documentation to better explain
what happens on close() and underline the need of closing with a
closed transaction if middleware such as PgBouncer is involved. I'd

(Continue reading)

headers

Daniele Varrazzo | 5 Aug 2010 02:32

Re: Psycopg 2.2.x issue with PgBouncer

On Mon, Aug 2, 2010 at 8:21 AM, Marko Kreen <markokr@...> wrote:
> On 8/2/10, Daniele Varrazzo <daniele.varrazzo@...> wrote:

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

I've checked and the autocommit mode creates no problem to pgbouncer.

If fog agrees to this change, I've prepared a patch
(http://tinyurl.com/36qufm4) dropping the currently failing attempt to
rollback and fixing the documentation for connection.close(), where
I've added:

Changed in version 2.2: previously an explicit ROLLBACK was issued by
Psycopg on close(). The command could have been sent to the backend at
an inappropriate time, so Psycopg currently relies on the backend to
implicitly discard uncommitted changes. Some middleware are known to
behave incorrectly though when the connection is closed during a
transaction (when status is STATUS_IN_TRANSACTION), e.g. PgBouncer
reports an unclean server and discards the connection. To avoid this
problem you can ensure to terminate the transaction with a
commit()/rollback()
before closing.

-- Daniele