Re: idle_in_transaction_timeout

Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On 2014-06-23 20:29:17 +0900, Fujii Masao wrote:
>> On Mon, Jun 23, 2014 at 7:48 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>>> On 2014-06-22 19:03:32 -0700, Kevin Grittner wrote:
>>>> Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>>>>
>>>>> I think we'll want a version of this that just fails the
>>>>> transaction once we have the infrastructure. So we should choose
>>>>> a name that allows for a complimentary GUC.

How about choosing a name for that, if we ever get there, where
what precedes _timeout describes what is being terminated?  Like
the idle transaction itself, rather than the session which shows as
being in "idle in transaction" status because of the idle
transaction which is using the session?

>>>> If we stick with the rule that what is to the left of _timeout is
>>>> what is being cancelled, the a GUC to cancel a transaction which
>>>> remains idle for too long could be called idle_transaction_timeout.
>>>>
>>>> Do you disagree with the general idea of following that pattern?
>>>
>>> I think that'd be rather confusing. For one it'd need to be
>>> idle_in_transaction_timeout

Why?  We're cancelling an idle transaction, not an "idle in
transaction", whatever that is.

>>> which already seems less clear (because the transaction belongs
>>> to idle)

I have no idea what that means.

>>> and for another that distinction seems to be to subtle for users.

The difference between an "idle in transaction session" and an
"idle transaction" is too subtle for someone preparing to terminate
one of those?

>>> The reason I suggested
>>> idle_in_transaction_termination/cancellation_timeout is that that maps
>>> nicely to pg_terminate/cancel_backend() and is rather descriptive.

I've always hated that naming because it is so confusing and
doesn't describe what is being terminated.  What makes it obvious
that "terminate" is something you do to a session rather than a
transaction?  What makes it obvious that "cancel" is something you
do to a transaction but not to a session?  Now if those were named
something like pg_rollback_backend_transaction() and
pg_close_backend_session() (or anything else where the names
actually made clear what was happening) then borrowing terminology
would be a stronger argument.  It still seems to me to be a weaker
argument than continuing the pattern we have for *_timeout GUCs.
though.

>> Maybe we can remove IIT_termination_timeout when we've implemented
>> IIT_cancellation_timeout. Right? I'm not sure if IIT_termination_timeout is
>> still useful even at that case.

In my experience, for the same reasons Robert gave much earlier in
the thread, if both were available the one which would be more
appropriate for cases I've seen would be the one that closed the
session with a FATAL message in the log.  If we were only going to
have one or the other, the one that terminated the session is the
one that *I* would prefer to see.

> A long idle in transaction state pretty much always indicates a
> problematic interaction with postgres.

True.  Which makes me wonder whether we shouldn't default this to
something non-zero -- even if it is 5 or 10 days.  It also gets me
thinking about whether a good follow-on patch would be a timeout
for prepared transactions.  I would have trouble counting how many
nasty production problems I've seen which would have been prevented
by having both time out after a few days.

BTW, since nobody has commented on the issue of the postgres_fdw
automatically exempting itself from the timeout, I will plan on
removing that when the naming argument reaches something resembling
a conclusion and I go to push this ... unless someone objects
before that.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company