Re: Why isn't stats_temp_directory automatically created? - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Why isn't stats_temp_directory automatically created?
Date
Msg-id 1603.1241383010@sss.pgh.pa.us
Whole thread Raw
In response to Re: Why isn't stats_temp_directory automatically created?  (Fujii Masao <masao.fujii@gmail.com>)
Responses Re: Why isn't stats_temp_directory automatically created?  (Magnus Hagander <magnus@hagander.net>)
Re: Why isn't stats_temp_directory automatically created?  (Bruce Momjian <bruce@momjian.us>)
List pgsql-hackers
Fujii Masao <masao.fujii@gmail.com> writes:
> Here is the revised patch; If stats_temp_directory indicates the symlink,
> we pursue the chain of symlinks and create the referenced directory.

I looked at this patch a bit.  I'm still entirely unconvinced that we
should be doing this at all --- if the directory is not there, there's
a significant probability that there's something wrong that is beyond
the backend's ability to understand or correct.  However, even ignoring
that objection, the patch is not ready to commit for a number of
reasons:

* Repeating the operation every time the stats file is written doesn't
seem like a particularly good idea; it eats cycles, and if the directory
disappears during live operation then there is *definitely* something
fishy going on.  Can't we fix it so that the work is only done when the
path setting changes?  (In principle you could do it in
assign_pgstat_temp_directory(), but I think something would be needed to
ensure that only the stats collector process actually tries to create
the directory.  Or maybe it would be simplest to try to run the code only
when we get a failure from trying to create the stats temp file.)

* I don't think the mkdir_p code belongs in fd.c.  It looks like
you copied-and-pasted it from initdb.c, which isn't any good either;
we don't want to maintain multiple copies of this.  Maybe a new
src/port/ file is indicated.

* elog(LOG) is not exactly an adequate response if the final chdir fails
--- you have just broken the process beyond recovery.  That alone may be
sufficient reason to reject the attempt to deal with symlinks.  As far
as pgstat_temp_directory is concerned, I'm not sure of the point of
making the GUC point to a symlink anyway --- if you have a GUC why not
just point it where you want the directory to be?
        regards, tom lane


pgsql-hackers by date:

Previous
From: justin
Date:
Subject: Re: windows doesn't notice backend death
Next
From: Tom Lane
Date:
Subject: Re: libpq is not thread safe