Re: [PATCH] lock_timeout and common SIGALRM framework

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/