Skip site navigation (1) Skip section navigation (2)

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

From: Boszormenyi Zoltan <zb(at)cybertec(dot)at>
To: Amit kapila <amit(dot)kapila(at)huawei(dot)com>
Cc: 'Tom Lane' <tgl(at)sss(dot)pgh(dot)pa(dot)us>, 'Robert Haas' <robertmhaas(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
Date: 2013-01-05 07:05:11
Message-ID: 50E7D0A7.3060400@cybertec.at (view raw or flat)
Thread:
Lists: pgsql-hackers
2013-01-05 05:58 keltezéssel, Amit kapila írta:
> On Friday, January 04, 2013 10:57 PM Boszormenyi Zoltan wrote:
>> Hi,
>> I am reviewing your patch.
> Thank you very much.
>
>> Since you are using a constant string, it would be a little faster
>> to use "sizeof(string)-1" as it can be computed at compile-time
>> and not run the strlen() all the time this code is reached.
> I have reffered the code just above for PG_TEMP_FILE_PREFIX in same function sendDir().
> I have that not only that place, but there other places where strlen is used for PG_TEMP_FILE_PREFIX.
> I think in this path, such optimization might not help much.
> However if you think we should take care of this, then I can find other places where similar change can be done
> to make it consistent?
>
>> 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.

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

I also tried to trigger one of the errors by creating the lock file manually.
You need an extra space between the "... retry" and "or ...":

+                               ereport(ERROR,
+ (errcode_for_file_access(),
+                                                errmsg("could not create lock file 
\"%s\": %m ", LockFileName),
+                                                errhint("May be too many concurrent edit 
into file happening, please wait!! and retry"
+                                                                "or .lock is file 
accidently left there please clean the file from config_dir")));

Best regards,
Zoltán Böszörményi

-- 
----------------------------------
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/



In response to

Responses

pgsql-hackers by date

Next:From: Boszormenyi ZoltanDate: 2013-01-05 08:37:08
Subject: Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
Previous:From: Amit kapilaDate: 2013-01-05 04:58:07
Subject: Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group