Re: [PATCH] lock_timeout and common SIGALRM framework

Boszormenyi Zoltan <zb(at)cybertec(dot)at> writes:
> Attached are the refreshed patches. InitializeTimeouts() can be called
> twice and PGSemaphoreTimedLock() returns bool now. This saves
> two calls to get_timeout_indicator().

I'm starting to look at this patch now. There are a number of cosmetic
things I don't care for, the biggest one being the placement of
timeout.c under storage/lmgr/. That seems an entirely random place,
since the functionality provided has got nothing to do with storage
let alone locks. I'm inclined to think that utils/misc/ is about
the best option in the existing backend directory hierarchy. Anybody
object to that, or have a better idea?

Another thing that needs some discussion is the handling of
InitializeTimeouts. As designed, I think it's completely unsafe,
the reason being that if a process using timeouts forks off another
one, the child will inherit the parent's timeout reasons and be unable
to reset them. Right now this might not be such a big problem because
the postmaster doesn't need any timeouts, but what if it does in the
future? So I think we should drop the base_timeouts_initialized
"protection", and that means we need a pretty consistent scheme for
where to call InitializeTimeouts. But we already have the same issue
with respect to on_proc_exit callbacks, so we can just add
InitializeTimeouts calls in the same places as on_exit_reset().

Comments?

I'll work up a revised patch and post it.

regards, tom lane