Re: idle_in_transaction_timeout - Mailing list pgsql-hackers

On 06/19/2014 05:42 PM, Kevin Grittner wrote:
> Abhijit Menon-Sen <ams@2ndquadrant.com> wrote:
>> At 2014-06-19 10:58:34 +0200, pavel.stehule@gmail.com wrote:
> 
>>> pgbouncer has idle_transaction_timeout defined some years
>>> without any problems, so we can take this design
>>>
>>> idle_transaction_timeout
>>
>> Let's steal the name too. :-)
>>
>> (I didn't realise there was precedent in pgbouncer, but given
>> that the name is in use already, it'll be less confusing to use
>> that name for Postgres as well.)
> 
> I'm not sure whether using the same name as pgbouncer for the same
> functionality makes it less confusing or might lead to confusion
> about which layer is disconnecting the client.  I'm leaning toward
> using the same name, but I'm really not sure.  Does anyone else
> want to offer an opinion?

I much prefer with "in" but it doesn't much matter.

> Somehow I had thought that pg_stat_activity didn't show a
> connection as "idle in transaction" as soon as BEGIN was run -- I
> thought some subsequent statement was needed to put it into that
> state; but I see that I was wrong about that.  As long as BEGIN
> causes a connection to show as "idle in transaction" I think the
> BEGIN should start the clock on this timeout, based on POLA.  

For me, this is a separate patch.  Whether it goes before or after this
one doesn't make a big difference, I don't think.

> It wouldn't bother me to see us distinguish among early transaction
> states, but I'm not sure whether it's really worth the effort.  We
> couldn't base it just on whether the transaction has acquired a
> snapshot, since it is not unusual for explicit LOCK statements at
> the front of the transaction to run before a snapshot is acquired,
> and we would want to terminate a transaction sitting idle and
> holding locks.
> 
> Also, it seems like most are ok with the FATAL approach (as in v1
> of this patch).  Does anyone want to make an argument against
> proceeding with that, in light of discussion so far?

From my view, we have these outstanding issues: - the name - begin-only transactions - aborted transactions

Those last two could arguably be considered identical.  If the client
issued a BEGIN, then the client is in a transaction and I don't think we
should report otherwise.  So then we need to decide why "idle in
transaction (aborted)" gets killed but "idle in transaction (initiated)"
doesn't.  The v1 patch doesn't currently kill the former but there was
question that it should.  I still believe it shouldn't.

Anyway, I'm willing to modify the patch in any way that consensus dictates.
-- 
Vik