Re: Concerns about statement-timeout patch - Mailing list pgsql-hackers

Added to TODO:
* Research interaction of setitimer() and sleep() used by statement_timeout

---------------------------------------------------------------------------

Tom Lane wrote:
> I've been trying to do some code review of the recent statement-timeout
> feature addition, and I've got some fairly serious concerns about it.
> 
> One problem that needs discussion is that the enable_sig_alarm and
> disable_sig_alarm calls were dropped into postgres.c at rather randomly
> chosen places.  The disable in particular is wrong because in the normal
> case it will occur after we have done transaction commit.  This creates
> a window between committing a command and reaching the disable call
> wherein the timeout interrupt could happen.  If it does happen, the
> net result will be that the client receives an error message, making
> it look like the command failed --- when in fact it was committed.
> 
> The simplest solution would be to move the calls into start_xact_command
> and finish_xact_command respectively.  However that would affect the
> semantics a little, in that for a querystring containing BEGIN and/or
> COMMIT commands, the timeout would be measured across subsets of the
> query string, not the whole string as now.  I am not sure if this is
> a problem or not; the existing semantics don't exactly match my idea
> of a "statement" timeout anyway.
> 
> Another possible objection is that end-of-transaction cleanup would
> not be counted in the statement timeout.  This does not bother me,
> since the cleanup should be quick, but maybe it would bother someone.
> (I would place the disable after DeferredTriggerEndQuery(), so that
> RI triggers are run before we disable the timeout.)
> 
> I am also concerned about the fact that the feature requires assuming
> that setitimer(ITIMER_REAL) plays nicely with sleep().  The documents
> I have say that on some platforms sleep() destroys any pending
> ITIMER_REAL setting.  We could perhaps fix that by adding a call to
> reset the end-of-statement timeout after every sleep() in the backend
> ... but that's obviously a fragile way to proceed.
> 
> Comments?
> 
>             regards, tom lane
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
> 
> http://www.postgresql.org/users-lounge/docs/faq.html
> 

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
359-1001+  If your life is a hard drive,     |  13 Roberts Road +  Christ can be your backup.        |  Newtown Square,
Pennsylvania19073