2013-01-14 07:47 keltezéssel, Amit kapila írta:
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 <z...@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?
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.
In that case (and also because the LWLock still serialize the whole procedure)
you can use GetTempFileName() on Windows and tempnam(3) for non-Windows.
GetTempFileName() and tempnam() return the constructed temporary file name
out of the directory and the filename prefix components.
The differences are:
1. For GetTempFileName(), you have to provide a buffer and it uses 3 bytes from
the prefix string. Always give you this path: dir\pfx<uuu>.TMP
2. tempnam() gives you a malloc()ed result buffer and uses at most 5 bytes from
the prefix string. According to the NOTES section in the man page, the specified
directory is only considered if $TMPDIR is not set in the environment or the
directory is not usable, like write is not permitted or the directory is
missing.
In this case. If TMPDIR not only is present and usable but on a different
filesystem
as the target config_dir/postgresql.auto.conf, rename() would return the EXDEV
error:
EXDEV The links named by new and old are on different file systems and the
implementation does not support links
between file systems.
Maybe mktemp(3) (not mk*s*temp!) instead of tempnam() is better here
because it always uses the specified template. You have to use you own
buffer for mktemp(). The BUGS section in man 3 mktemp says:
BUGS
Never use mktemp(). Some implementations follow 4.3BSD and replace XXXXXX by the
current process ID and a single
letter, so that at most 26 different names can be returned. Since on the one hand
the names are easy to guess, and
on the other hand there is a race between testing whether the name exists and
opening the file, every use of
mktemp() is a security risk. The race is avoided by mkstemp(3).
I think we don't really care about the security risk about the file name
being easy to guess and there is no race condition because of the LWLock.
With GetTempFileName() and mktemp(), you can have a lot of common code,
like pass the same 3-letter prefix and preallocate or statically declare the
output buffer.
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.
No objection.
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/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
