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