Re: [PATCH] lock_timeout and common SIGALRM framework

Excerpts from Boszormenyi Zoltan's message of mar jun 19 12:44:04 -0400 2012:

> OK, all 4 Check* functions are now moved back into proc.c,
> nothing outside of timeout.c touches anything in it. New patches
> are attached.

Yeah, I like this one better, thanks.

It seems to me that the "check" functions are no longer "check" anymore,
right?  I mean, they don't check whether the time has expired.  It can
be argued that CheckDeadLock is well named, because it does check
whether there is a deadlock before doing anything else; but
CheckStandbyTimeout is no longer a check -- it just sends a signal.
Do we need to rename these functions?

Why are you using the deadlock timeout for authentication?  Wouldn't it
make more sense to have a separate TimeoutName, just to keep things
clean?

The "NB:" comment here doesn't seem to be useful anymore:
+ /*****************************************************************************
+  * Init, Destroy and Check functions for different timeouts.
+  *
+  * NB: all Check* functions are run inside a signal handler, so be very wary
+  * about what is done in them or in called routines.
+  *****************************************************************************/


In base_timeouts you don't initialize fin_time for any of the members.
This is probably unimportant but then why initialize start_time?

-- 
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support