Here is a revised version of the timeout-infrastructure patch. I whacked it around quite a bit, notably:
* I decided that the most convenient way to handle the initialization
issue was to combine establishment of the signal handler with resetting
of the per-process variables. So handle_sig_alarm is no longer global,
and InitializeTimeouts is called at the places where we used to do
"pqsignal(SIGALRM, handle_sig_alarm);". I believe this is correct
because any subprocess that was intending to use SIGALRM must have
called that before establishing any timeouts.
* BTW, doing that exposed the fact that walsender processes were failing
to establish a SIGALRM signal handler, which is a pre-existing bug,
because they run a normal authentication transaction during InitPostgres
and hence need to be able to cope with deadlock and statement timeouts.
I will do something about back-patching a fix for that.
* I ended up putting the RegisterTimeout calls for DEADLOCK_TIMEOUT
and STATEMENT_TIMEOUT into InitPostgres, ensuring that they'd get
done in walsender and autovacuum processes. I'm not totally satisfied
with that, but for sure it didn't work to only establish them in
regular backends.
* I didn't like the "TimeoutName" nomenclature, because to me "name"
suggests that the value is a string, not just an enum. So I renamed
that to TimeoutId.
* I whacked around the logic in timeout.c a fair amount, because it
had race conditions if SIGALRM happened while enabling or disabling
a timeout. I believe the attached coding is safe, but I'm not totally
happy with it from a performance standpoint, because it will do two
setitimer calls (a disable and then a re-enable) in many cases where
the old code did only one.
I think what we ought to do is go ahead and apply this, so that we
can have the API nailed down, and then we can revisit the internals
of timeout.c to see if we can't get the performance back up.
It's clearly a much cleaner design than the old spaghetti logic in
proc.c, so I think we ought to go ahead with this independently of
whether the second patch gets accepted.
I haven't really looked at the second patch yet, but at minimum that
will need some rebasing to match the API tweaks here.
regards, tom lane
binwIymnjnW5K.bin
Description: 1-timeout-framework-v16.patch.gz
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
