I cleaned up the framework patch a bit. My version's attached. Mainly,
returning false for failure in some code paths that are only going to
have the caller elog(FATAL) is rather pointless -- it seems much better
to just have the code itself do the elog(FATAL). In any case, I
searched for reports of those error messages being reported in the wild
and saw none.
There are other things but they are in the nitpick department. (A
reference to "->check_timeout" remains that needs to be fixed too).
There is one thing that still bothers me on this whole framework patch,
but I'm not sure it's easily fixable. Basically the API to timeout.c is
the whole list of timeouts and their whole behaviors. If you want to
add a new one, you can't just call into the entry points, you have to
modify timeout.c itself (as well as timeout.h as well as whatever code
it is that you want to add timeouts to). This may be good enough, but I
don't like it. I think it boils down to proctimeout.h is cheating.
The interface I would actually like to have is something that lets the
modules trying to get a timeout register the timeout-checking function
as a callback. API-wise, it would be much cleaner. However, I'm not
raelly sure that code-wise it would be any cleaner at all. In fact I
think it'd be rather cumbersome. However, if you could give that idea
some thought, it'd be swell.
I haven't looked at the second patch at all yet.
--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
