Re: [PATCH] lock_timeout and common SIGALRM framework - Mailing list pgsql-hackers

From Alvaro Herrera
Subject Re: [PATCH] lock_timeout and common SIGALRM framework
Date
Msg-id 1342039290-sup-9091@alvh.no-ip.org
Whole thread Raw
In response to Re: [PATCH] lock_timeout and common SIGALRM framework  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: [PATCH] lock_timeout and common SIGALRM framework  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
Excerpts from Tom Lane's message of mié jul 11 15:47:47 -0400 2012:
>
> Boszormenyi Zoltan <zb@cybertec.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?

I agree with the proposed new location.

> 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().

I do agree that InitializeTimeouts is not optimally placed.  We
discussed this upthread.

Some of the calls of on_exit_reset() are placed in code that's about to
die.  Surely we don't need InitializeTimeouts() then.  Maybe we should
have another routine, say InitializeProcess (noting we already
InitProcess so maybe some name would be good), that calls both
on_exit_reset and InitializeTimeouts.

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


pgsql-hackers by date:

Previous
From: Peter Eisentraut
Date:
Subject: Re: has_language_privilege returns incorrect answer for non-superuser
Next
From: Peter Eisentraut
Date:
Subject: Re: HTTP API experimental implementation