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 <and...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
