Re: idle_in_transaction_timeout

From: Kevin Grittner <kgrittn(at)ymail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, Vik Fearing <vik(dot)fearing(at)dalibo(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Josh Berkus <josh(at)agliodbs(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>
Subject: Re: idle_in_transaction_timeout
Date: 2014-06-29 16:32:07
Message-ID: 1404059527.74799.YahooMailNeo@web122303.mail.ne1.yahoo.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Wed, Jun 25, 2014 at 11:40 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

>> Fujii Masao <masao(dot)fujii(at)gmail(dot)com> writes:
>>> Why is IIT timeout turned on only when send_ready_for_query is true?
>>> I was thinking it should be turned on every time a message is received.
>>> Imagine the case where the session is in idle-in-transaction state and
>>> a client gets stuck after sending Parse message and before sending Bind
>>> message. This case would also cause long transaction problem and should
>>> be resolved by IIT timeout. But IIT timeout that this patch adds cannot
>>> handle this case because it's enabled only when send_ready_for_query is
>>> true. Thought?
>>
>> I think you just moved the goalposts to the next county.
>>
>> The point of this feature, AFAICS, is to detect clients that are failing
>> to issue another query or close their transaction as a result of bad
>> client logic.  It's not to protect against network glitches.
>
> Hmm, so there's no reason a client, after sending one protocol
> message, might not pause before sending the next protocol message?
> That seems like a surprising argument.  Someone couldn't Parse and
> then wait before sending Bind and Execute, or Parse and Bind and then
> wait before sending Execute?
>
>
>> Moreover, there would be no way to implement a timeout like that without
>> adding a gettimeofday() call after every packet receipt, which is overhead
>> we do not need and cannot afford.  I don't think this feature should add
>> *any* gettimeofday's beyond the ones that are already there.
>
> That's an especially good point if we think that this feature will be
> enabled commonly or by default - but as Fujii Masao says, it might be
> tricky to avoid.  :-(

I think that this patch, as it stands, is a clear win if the
postgres_fdw part is excluded.  Remaining points of disagreement
seem to be the postgres_fdw, whether a default value measured in
days might be better than a default of off, and whether it's worth
the extra work of covering more.  The preponderance of opinion
seems to be in favor of excluding the postgres_fdw changes, with
Tom being violently opposed to including it.  I consider the idea
of the FDW ignoring the server setting dead.  Expanding the
protected area of code seems like it would be sane to ask whoever
wants to extend the protection in that direction to propose a
patch.  My sense is that there is more support for a default of a
small number of days than a default of never, but that seems far
from settled.

I propose to push this as it stands except for the postgres_fdw
part.  The default is easy enough to change if we reach consensus,
and expanding the scope can be a new patch in a new CF.
Objections?

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2014-06-29 16:53:56 Re: idle_in_transaction_timeout
Previous Message Tom Lane 2014-06-29 16:30:10 Re: Patch to send transaction commit/rollback stats to the stats collector unconditionally.