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(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support