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

From: Amit kapila <amit(dot)kapila(at)huawei(dot)com>
To: Boszormenyi Zoltan <zb(at)cybertec(dot)at>
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-14 06:47:02
Message-ID: 6C0B27F7206C9E4CA54AE035729E9C383BEB4493@szxeml509-mbx
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sunday, January 13, 2013 2:45 PM Boszormenyi Zoltan wrote:
2013-01-13 05:49 keltezéssel, Amit kapila írta:
> On Sunday, January 13, 2013 12:41 AM Tom Lane wrote:
> Boszormenyi Zoltan <zb(at)cybertec(dot)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?

I think for Windows exactly same behavior API might not exist, we might need to use GetTempFileName.
It will generate some different kind of name, which means cleanup also need to take care of different kind of names.
So if this is right, I think it might not be very appropriate to use it.

Please point me if I am wrong as I have not used it.

> 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 PostmasterMain(), we already call RemovePGTempFiles(); So I think we can delete this tmp file at
that place in PostmasterMain().
I shall do this, if nobody raise objection about it.

With Regards,
Amit Kapila.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message 李海龙 2013-01-14 07:18:18 Re: I s this a bug of spgist index in a heavy write condition?
Previous Message Gurjeet Singh 2013-01-14 03:42:09 Re: count(*) of zero rows returns 1