Re: [PATCH] lock_timeout and common SIGALRM framework

From: Boszormenyi Zoltan <zb(at)cybertec(dot)at>
To: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Cc: 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-06-19 17:01:13
Message-ID: 4FE0B059.5020503@cybertec.at
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

2012-06-19 18:44 keltezéssel, Boszormenyi Zoltan írta:
> 2012-06-19 16:54 keltezéssel, Alvaro Herrera írta:
>> Excerpts from Boszormenyi Zoltan's message of mar jun 19 04:44:35 -0400 2012:
>>> 2012-06-18 19:46 keltezéssel, Alvaro Herrera írta:
>>>> Excerpts from Boszormenyi Zoltan's message of vie may 11 03:54:13 -0400 2012:
>>>>> Hi,
>>>>>
>>>>> another rebasing and applied the GIT changes in
>>>>> ada8fa08fc6cf5f199b6df935b4d0a730aaa4fec to the
>>>>> Windows implementation of PGSemaphoreTimedLock.
>>>> Hi,
>>>>
>>>> I gave the framework patch a look. One thing I'm not sure about is the
>>>> way you've defined the API. It seems a bit strange to have a nice and
>>>> clean, generic interface in timeout.h; and then have the internal
>>>> implementation of the API cluttered with details such as what to do when
>>>> the deadlock timeout expires. Wouldn't it be much better if we could
>>>> keep the details of CheckDeadLock itself within proc.c, for example?
>>> Do you mean adding a callback function argument to for enable_timeout()
>>> would be better?
>> Well, that's not exactly what I was thinking, but maybe your idea is
>> better than what I had in mind.
>>
>>>> Same for the other specific Check*Timeout functions. It seems to me
>>>> that the only timeout.c specific requirement is to be able to poke
>>>> base_timeouts[].indicator and fin_time. Maybe that can get done in
>>>> timeout.c and then have the generic checker call the module-specific
>>>> checker.
>>> Or instead of static functions, Check* functions can be external
>>> to timeout.c. It seemed to be a good idea to move all the timeout
>>> related functions to timeout.c.
>> Yeah, except that they play with members of the timeout_params struct,
>> which hopefully does not need to be exported. So if you can just move
>> (that is, put back in their original places) the portions that do not
>> touch that strcut, it'd be great.
>
> OK, all 4 Check* functions are now moved back into proc.c,
> nothing outside of timeout.c touches anything in it. New patches
> are attached.

OK, one last post for today. The #included headers are cleaned up
in timeout.c, miscadmin.h and storage/standby.h are not needed.

--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
http://www.postgresql.at/

Attachment Content-Type Size
1-timeout-framework-v7.1.patch text/x-patch 45.4 KB
2-lock_timeout-v7.1.patch text/x-patch 46.4 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2012-06-19 17:19:04 Re: [PATCH] lock_timeout and common SIGALRM framework
Previous Message Tom Lane 2012-06-19 16:48:39 Near-duplicate RI NO ACTION and RESTRICT triggers