Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
От | Boszormenyi Zoltan |
---|---|
Тема | Re: Proposal for Allow postgresql.conf values to be changed via SQL [review] |
Дата | |
Msg-id | 50F27B1B.70203@cybertec.at обсуждение исходный текст |
Ответ на | Re: Proposal for Allow postgresql.conf values to be changed via SQL [review] (Amit kapila <amit.kapila@huawei.com>) |
Ответы |
Re: Proposal for Allow postgresql.conf values to be
changed via SQL [review]
(Amit kapila <amit.kapila@huawei.com>)
|
Список | pgsql-hackers |
2013-01-13 05:49 keltezéssel, Amit kapila írta: > On Sunday, January 13, 2013 12:41 AM Tom Lane wrote: > Boszormenyi Zoltan <zb@cybertec.at> writes: >>> No, I mean the reaper(SIGNAL_ARGS) function in >>> src/backend/postmaster/postmaster.c where your patch has this: >>> *** a/src/backend/postmaster/postmaster.c >>> --- b/src/backend/postmaster/postmaster.c >>> *************** >>> *** 2552,2557 **** reaper(SIGNAL_ARGS) >> I have not been following this thread, but I happened to see this bit, >> and I have to say that I am utterly dismayed that anything like this is >> even on the table. This screams overdesign. We do not need a global >> lock file, much less postmaster-side cleanup. All that's needed is a >> suitable convention about temp file names that can be written and then >> atomically renamed into place. If we're unlucky enough to crash before >> a temp file can be renamed into place, it'll just sit there harmlessly. > I can think of below ways to generate tmp file > 1. Generate session specific tmp file name during operation. This will be similar to what is > currently being done in OpenTemporaryFileinTablespace() to generate a tempfilename. > 2. Generate global tmp file name. For this we need to maintain global counter. > 3. Always generate temp file with same name "postgresql.auto.conf.tmp", as at a time only one > session is allowed to operate. What's wrong with mkstemp("config_dir/postgresql.auto.conf.XXXXXX") that returns a file descriptor for an already created file with a unique name? Note that the working directory of PostgreSQL is $PGDATA so "config_dir" and files under that can be accessed with a relative path. But a cleanup somewhere is needed to remove the stale temp files. I think it's enough at postmaster startup. A machine that's crashing so often that the presence of the stale temp files causes out of disk errors would surely be noticed much earlier. > > In any approach, there will be chance that temp files will remain if server crashes during this command execution > which can lead to collision of temp file name next time user tries to use SET persistent command. mkstemp() replaces the "XXXXXX" suffix with a unique hexadecimal suffix so there will be no collision. > An appropriate error will be raised and user manually needs to delete that file. > > > >>>>> I just noticed that you are using "%m" in format strings twice. man 3 printf says: >>>>> m (Glibc extension.) Print output of strerror(errno). No argument is required. >>>>> This doesn't work anywhere else, only on GLIBC systems, it means Linux. >>>>> I also like the brevity of this extension but PostgreSQL is a portable system. >>>>> You should use %s and strerror(errno) as argument explicitly. >>>> %m is used at other places in code as well. >> As far as that goes, %m is appropriate in elog/ereport (which contain >> special support for it), but not anywhere else. > In the patch, it's used in ereport, so I assume it is safe and patch doesn't need any change for %m. OK. > > > With Regards, > Amit Kapila. > > -- ---------------------------------- 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/
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Pavel StehuleДата:
Сообщение: is it bug? - printing boolean domains in sql/xml function