From: Tom Lane <tgl@sss.pgh.pa.us>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: pgsql-hackers@postgresql.org, Fabien COELHO <coelho@cri.ensmp.fr>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: David Steele <david@pgmasters.net>
Cc: Bossart, Nathan <bossartn@amazon.com>
Cc: Thomas Munro <thomas.munro@gmail.com>
Subject: Re: pg11+: pg_ls_*dir LIMIT 1: temporary files .. not closed at end-of-transaction
Date: Sat, 28 Mar 2020 15:07:55 -0400
Message-ID: <28906.1585422475@sss.pgh.pa.us> (raw)
In-Reply-To: <20200328183904.GI20103@telsasoft.com>
References: <27334.1583692669@sss.pgh.pa.us>
<20200308191456.GD1357@telsasoft.com>
<5679.1583696409@sss.pgh.pa.us>
<20200311111921.GQ29065@telsasoft.com>
<21724.1583955158@sss.pgh.pa.us>
<20200312121156.GB29065@telsasoft.com>
<20200316155306.GM26184@telsasoft.com>
<3061.1584409130@sss.pgh.pa.us>
<20200317020017.GT26184@telsasoft.com>
<24244.1585415634@sss.pgh.pa.us>
<20200328183904.GI20103@telsasoft.com>
Justin Pryzby <pryzby@telsasoft.com> writes:
> On Sat, Mar 28, 2020 at 01:13:54PM -0400, Tom Lane wrote:
>> so I propose that we fix these directory-scanning functions to silently
>> ignore ENOENT failures from stat(). Are there any for which we should not do
>> that?
> Maybe we should lstat() the file to determine if it's a dangling link; if
> lstat() fails, then skip it. Currently, we use stat(), which shows metdata of
> a link's *target*. Maybe we'd change that.
Hm, good point that ENOENT could refer to a symlink's target. Still,
I'm not sure it's worth going out of our way to disambiguate that,
given that these directories aren't really supposed to contain symlinks.
(And on the third hand, if they aren't supposed to, then maybe these
functions needn't look through any symlinks? In which case just
substituting lstat for stat would resolve the ambiguity.)
> Note that I have a patch which generalizes pg_ls_dir_files and makes
> pg_ls_dir() a simple wrapper, so if that's pursued, they would behave the same
> unless I add another flag to do otherwise (but behaving the same has its
> merits). It already uses lstat() to show links to dirs as isdir=no, which was
> needed to avoid recursing into links-to-dirs in the new helper function
> pg_ls_dir_recurse(). https://commitfest.postgresql.org/26/2377/
I think we need a back-patchable fix for the ENOENT failure, seeing that
we back-patched the new regression test; intermittent buildfarm failures
are no fun in any branch. So new functions aren't too relevant here,
although it's fair to look ahead at whether the same behavior will be
appropriate for them.
regards, tom lane
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, pryzby@telsasoft.com, coelho@cri.ensmp.fr, alvherre@2ndquadrant.com, david@pgmasters.net, bossartn@amazon.com, thomas.munro@gmail.com
Subject: Re: pg11+: pg_ls_*dir LIMIT 1: temporary files .. not closed at end-of-transaction
In-Reply-To: <28906.1585422475@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox