Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]

On Sat, Jan 05, 2013 at 08:05:11AM +0100, Boszormenyi Zoltan wrote:
> 2013-01-05 05:58 keltez?ssel, Amit kapila ?rta:
>> On Friday, January 04, 2013 10:57 PM Boszormenyi Zoltan wrote:
>>> In create_conf_lock_file():
>>
>>> Can't we add a new LWLock and use it as a critical section instead
>>> of waiting for 10 seconds? It would be quite surprising to wait
>>> 10 seconds when accidentally executing two SET PERSISTENT
>>> statements in parallel.
>> Yes, you are right adding a new LWLock will avoid the use of sleep.
>> However I am just not sure if for only this purpose we should add a new LWLock?
>>
>> Similar logic is used in CreateLockFile() for postmaster file but it doesn't use sleep.
>> How about reducing the time of sleep or removing sleep, in that user might get
>> error and he need to retry to get his command successful?
>
> I think removing the loop entirely is better.

Agreed. A new LWLock with a narrow purpose is fine. CreateLockFile() happens
early, before shared memory is available. Its approach is not a relevant
precedent for code that runs later.

> However, the behaviour should be discussed by a wider audience:
> - the backends should wait for each other just like other statements
> that fight for the same object (in which case locking is needed), or
> - throw an error immediately on conflicting parallel work

All other things being equal, the first choice sounds better. (I don't think
the decision is essential to the utility of this feature.)