Re: [PATCH] lock_timeout and common SIGALRM framework

From: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Boszormenyi Zoltan <zb(at)cybertec(dot)at>, Marc Cousin <cousinmarc(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Hans-Juergen Schoenig <hs(at)cybertec(dot)at>, Ants Aasma <ants(at)cybertec(dot)at>, Robert Haas <robertmhaas(at)gmail(dot)com>
Subject: Re: [PATCH] lock_timeout and common SIGALRM framework
Date: 2012-07-11 20:52:29
Message-ID: 1342039290-sup-9091@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


Excerpts from Tom Lane's message of mié jul 11 15:47:47 -0400 2012:
>
> 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?

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2012-07-11 20:58:21 Re: HTTP API experimental implementation
Previous Message Shaun Thomas 2012-07-11 20:47:21 Re: DELETE vs TRUNCATE explanation