Re: Rollback on close [Was: Fwd: [DB-SIG] conn.close() idempotence]

On Wed, Oct 19, 2011 at 3:59 PM, Daniele Varrazzo
<daniele(dot)varrazzo(at)gmail(dot)com> wrote:
> On Wed, Oct 19, 2011 at 1:12 PM, Marko Kreen <markokr(at)gmail(dot)com> wrote:
>> First, "in transaction" is not enough, it must check if connections
>> is "idle in transaction" and no query has been sent.
>
> Question: is for the middleware different if a connection is idle in
> transaction with or without a query has been sent to the database?

It does not make sense to send rollback packet in the middle of copy
or long query, or even when the query resultset has not fully arrive yet,
as you will not know how big it is.

> However it's easy to check the "idle in transaction" state in the
> connection; and actually, because it's us who send the commands, we
> could also detect about any sent query.

Yes.

> Your question also makes me think about what should happen if a
> close() is issued in a separate thread while a query is running... but
> this should be just handled by the serialization code in the
> transaction, i.e. the close should wait until the query has finished.

Bad idea, the query resultset may be big.

>> Secondly, I think there are two types of code to consider:
>>
>> - Sloppy code - eg read-only web page that does
>>
>>  db = connect()
>>  curs.execute('select ...')
>>  curs.execute('select ...')
>>  db.close()
>>
>> - Robust code, where in-transaction-close means
>>  problem, and it wants to get rid of connection
>>  without touching network.
>>
>> Although I understand the urge to optimize for first case,
>> you take away the possibility of robust code to behave well.
>
> What is an example of situation where a close in transaction without
> rollback is a better option than rolling back too?

- incoming copy, incoming big resultset
- network down (why wait for timeout on every connection)
- signal (need fast shutdown, connection may be in unstable state)
- closing from other thread, without locking
(some sort of disaster/quick shutdown scenario)

Actually, now that I think about it, the *only* scenario
where the rollback is desirable is above-mentioned
sloppy code with middleware (eg. pgbouncer)
that drops connections when closed in middle of tx.

In every other case I can think of, the ROLLBACK
will cause problems.

>> So if you really want to restore the rollback-on-close
>> behaviour, at least make it configurable.
>
> I'm more for making a decision instead of leaving too many things to
> be configured, so if we deem that closing without explicit rollback is
> still a better solution I'm fine with leaving it this way and suggest
> users to write less sloppy code. I.E. I would *not* like to add an
> option such as conn.close(rollback=False).

But would you like to document that:

"To reliable close connection, do os.close(db.fileno()) before db.close()"

?

>> OTOH, as the lightweight .close() is only problematic
>> with middleware, it seems to hint that this idle-in-tx
>> check should be moved into middleware, and psycopg
>> should not need to worry about it..
>
> Well, you know the middleware much better than me: I was assuming that
> if pgpool discards connection returned idle in transaction to the pool
> you have very strong reasons :) I just want to optimize the
> communication between the driver and the middleware: what do you think
> the "protocol" between psycopg and pgpool should be?

Thats easy - do not rollback. Only reason for rollback is to work around
current middleware, so if current middleware behaviour is bad,
it should be fixed. Middleware should be transparent, client code
should not need to have workarounds.

Btw, I know pgbouncer, I do not know pgpool. I saw a mention of pgpool
above and guessed it behaves the same.

PgBouncer drops server connection if client closes in the middle of transaction,
on the principle of "in-tx close is sign of problem, lets propagate it further".
So the only question for me is - should I fix pgbouncer to keep server
connection in such situation? The answer seems to be "yes" if I think
about maximal pooling efficiency, but "no" if I think that only use would
be to keep crap code running and potential problems in disaster situations.

Eg, pgbouncer has some logic for disaster situations that depends
on server connections going away from pool (fast_fail).
But the fix would keep server connection around longer.
OTOH, when strictly idle-in-tx, the client-side problem
should not say anything about server connection.

--
marko