On 02/11/2013 06:33 AM, Boszormenyi Zoltan wrote:
> 2013-02-11 15:25 keltezéssel, Andres Freund írta:
>> On 2013-02-11 15:21:13 +0100, Boszormenyi Zoltan wrote:
>>> 2013-01-24 18:02 keltezéssel, Tom Lane írta:
>>>> Andres Freund <andres@2ndquadrant.com> writes:
>>>>> On 2013-01-24 11:22:52 -0500, Tom Lane wrote:
>>>>>> Say again? Surely the temp file is being written by whichever
>>>>>> backend
>>>>>> is executing SET PERSISTENT, and there could be more than one.
>>>>> Sure, but the patch acquires SetPersistentLock exlusively beforehand
>>>>> which seems fine to me.
>>>> Why should we have such a lock? Seems like that will probably
>>>> introduce
>>>> as many problems as it fixes. Deadlock risk, blockages, etc. It is
>>>> not
>>>> necessary for atomicity, since rename() would be atomic already.
>>> There is a problem when running SET PERSISTENT for different GUCs
>>> in parallel. All happen to read the same original file, and only one
>>> setting ends up in the result if you rely only on the rename() being
>>> atomic.
>>> The LWLock provides the serialization for that problem.
>> Tom was voting for one-setting-per-file, in that case the problem
>> doesn't exist.
>
> I voted for the one-file approach and was arguing from the POV
> of the current implementation.
I thought we discussed this ad naseum, and decided to go with the
one-single-file approach for the first round, since we already had an
implementation for that. I still think that's the right approach to take
with this patch; if it doesn't work out, we can go do
one-file-per-setting in 9.4.
--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com
