Horton
Menu

Thread

  1. Fwd: Re: A new look at old NFS readdir() problems?

    Larry Rosenman <ler@lerctr.org> — 2025-01-02T20:26:28Z

    @Tom Lane: This is what Rick Macklem (NFS dev on FreeBSD) has to say on 
my issue.


-------- Original Message --------
Subject: Re: A new look at old NFS readdir() problems?
Date: 01/02/2025 10:08 am
 From: Rick Macklem <rick.macklem@gmail.com>
To: Thomas Munro <tmunro@freebsd.org>
Cc: Rick Macklem <rmacklem@freebsd.org>, Larry Rosenman <ler@lerctr.org>

On Thu, Jan 2, 2025 at 2:50 AM Thomas Munro <tmunro@freebsd.org> wrote:
> 
> CAUTION: This email originated from outside of the University of 
> Guelph. Do not click links or open attachments unless you recognize the 
> sender and know the content is safe. If in doubt, forward suspicious 
> emails to IThelp@uoguelph.ca.
> 
> 
> Hi Rick
> CC: ler
> 
> I hope you don't mind me reaching out directly, I just didn't really
> want to spam existing bug reports without sufficient understanding to
> actually help yet...  but I figured I should get in touch and see if
> you have any clues or words of warning, since you've worked on so much
> of the NFS code.  I'm a minor FBSD contributor and interested in file
> systems, but not knowledgeable about NFS; I run into/debug/report a
> lot of file system bugs on a lot of systems in my day job on
> databases.  I'm interested to see if I can help with this problem.
> Existing ancient report and interesting email:
> 
> https://lists.freebsd.org/pipermail/freebsd-fs/2014-October/020155.html
> https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=57696
> 
> What we ran into is not the "bad cookie" state, which doesn't really
> seem to be recoverable in general, from what I understand (though the
> FreeBSD code apparently would try, huh).  It's a simple case where the
> NFS client requests a whole directory with a large READDIR request,
> and then tries to unlink all the files in a traditional
> while-readdir()-unlink() loop that works on other systems.
In general, NFS is not a POSIX compliant file system, due to its 
protocol
design. The above is one example. The only "safe" way is to opendir() or
rewinddir() after every removal.

The above usually works (and always worked for UFS long ago) because
the directory offset cookies for subsequent entries in the directory 
after
the entry unlinked happened to "still be valid". That is no longer true
for FreeBSD's UFS nor for many other file systems that can be exported.

If the client reads the entire directory in one READDIR, then it is 
fine,
since it has no need to the directory offset cookies. However, there is
a limit to how much a single READDIR can do (these days for NFSv4.1/4.2,
it could be raised to just over 1Mbyte, however FreeBSD limits it to 8K 
at
the moment).

Another way to work around the problem is to read the entire directory
into the client via READDIRs before starting to do the unlinks.
The opendir()/readdir() code in libc could be hacked to do that,
but I have never tried to push such a patch into FreeBSD.
(It would be limited by how much memory can be malloc()'d, that
is pretty generous compared to even large directorys with 10s of
thousand entries.)

The above is true for all versions of NFS up to NFSv4.2, which is
the current one and unless some future version of NFS does READDIR
differently (I won't live long enough to see this;-), it will always
be the case.

If my comment above was not clear, the following encoding is the "safe"
way to remove all entries in a directory.

do {
       dir = opendir("X");
        dp = readdir(dir);
        if (dp != NULL)
             unlink(dp->d_name);
         close(dir);
} while (dp != NULL);

In theory, the directory_offset_cookie was supposed to handle this, but 
it
has never worked correctly, for a couple of reasons.
1 - RFC1813 (the NFSv3 one) did not describe the cookie verifier 
correctly.
      It should only change when cookies for extant entries change. The
description
      suggested it should change whenever an entry is deleted, since that 
cookie
      is no longer valid.
2 - #1 only works if directory offset cookies for other entries in the 
directory
     do not change when an entry is deleted. This used to be the case for 
UFS,
     but was broken in FreeBSD when a commit many years ago optimized
     ufs_readdir() to compress out invalid entries. Doing this changes 
the
     directory offset cookies every time an entry is deleted at the 
beginning
     of a directory block.

rick
>  On FreeBSD
> it seems to clobber its own directory cache, make extra unnecessary
> READDIR requests, and skip some of the files.  Or maybe I have no idea
> what's going on and this is a hopelessly naive question and mission
> :-)
> 
> Here's what we learned so far starting from Larry's report:
> 
> https://www.postgresql.org/message-id/flat/04f95c3c13d4a9db87b3ac082a9f4877%40lerctr.org
> 
> Note that this issue has nothing to do with "bad cookie" errors (I
> doubt the server I'm talking to even implements that -- instead it
> tries to have cookies that are persistent/stable).
> 
> Also, while looking into this and initially suspecting cookie
> stability bugs (incorrectly), I checked a bunch of local file systems
> to understand how their cookies work, and I think I found a related
> problem when FreeBSD exports UFS, too.  I didn't repro this with NFS
> but it's clearly visible from d_off locally with certain touch, rm
> sequences.  First, let me state what I think the cookie should be
> trying to achieve, on a system that doesn't implement "bad cookie" but
> instead wants cookies that are persistent/always valid:  if you make a
> series of READDIR requests using the cookie from the final entry of
> the previous response, it should be impossible to miss any entry that
> existed before your first call to readdir(), and impossible to see any
> entry twice.  It is left undefined whether entries created after that
> time are visible, since anything else would require unbounded time or
> space via locks or multi-version magic (= isolation problems from
> database-land).
> 
> Going back to the early 80s, Sun UFS looks good (based on illumos
> source code) because it doesn't seem to move entries after they are
> created.  That must have been the only file system when they invented
> VFS and NFS.  Various other systems since have been either complex but
> apparently good (ZFS/ZAP cursors can tolerate up to 2^16 hash
> collisions which I think we can call statistically impossible, XFS
> claims to be completely stable though I didn't understand fully why,
> BTRFS assigns incrementing numbers that will hopefully not wrap, ...),
> or nearly-good-enough-but-ugh (ext4 uses hashes like ZFS but
> apparently fails with ELOOP on hash collisions?).  I was expecting
> FreeBSD UFS to be like Sun UFS but I don't think it is!  In the UFS
> code since at least 4.3BSD (but apparently not in the Sun version,
> forked before or removed later?), inserting a new entry can compact a
> directory page, which moves the offset of a directory entry lower.
> AFAICS we can't move an entry lower, or we risk skipping it in NFS
> readdir(), and we can't move it higher, or we risk double-reporting it
> in readdir().  Or am I missing something?
> 
> Thanks for reading and happy new year,
> 
> Thomas Munro

-- 
Larry Rosenman                     http://www.lerctr.org/~ler
Phone: +1 214-642-9640                 E-Mail: ler@lerctr.org
US Mail: 13425 Ranch Road 620 N, Apt 718, Austin, TX 78717-1010

  2. Re: Fwd: Re: A new look at old NFS readdir() problems?

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-01-02T20:48:53Z

    Larry Rosenman <ler@lerctr.org> writes:
> @Tom Lane: This is what Rick Macklem (NFS dev on FreeBSD) has to say on 
> my issue.

Thanks for reaching out to him.  So if I'm reading this correctly,
there's little point in filing a FreeBSD bug because it'll be
dismissed as unfixable.

This leaves us in rather a nasty position.  Sure, we could rewrite
rmtree() as Thomas suggested upthread, but I'm still of the opinion
that that's smearing lipstick on a pig.  rmtree() is the least of
our worries: it doesn't need to expect that anybody else will be
modifying the target directory, plus it can easily restart its scan
without complicated bookkeeping.  I doubt we can make such an
assumption for all our uses of readdir(), or that it's okay to
miss or double-process files in every one of them.

I'm still of the opinion that the best thing to do is disclaim
safety of storing a database on NFS.

			regards, tom lane

  3. Re: Fwd: Re: A new look at old NFS readdir() problems?

    Bruce Momjian <bruce@momjian.us> — 2025-01-02T20:50:43Z

    On Thu, Jan  2, 2025 at 03:48:53PM -0500, Tom Lane wrote:
> Larry Rosenman <ler@lerctr.org> writes:
> > @Tom Lane: This is what Rick Macklem (NFS dev on FreeBSD) has to say on 
> > my issue.
> 
> Thanks for reaching out to him.  So if I'm reading this correctly,
> there's little point in filing a FreeBSD bug because it'll be
> dismissed as unfixable.
> 
> This leaves us in rather a nasty position.  Sure, we could rewrite
> rmtree() as Thomas suggested upthread, but I'm still of the opinion
> that that's smearing lipstick on a pig.  rmtree() is the least of
> our worries: it doesn't need to expect that anybody else will be
> modifying the target directory, plus it can easily restart its scan
> without complicated bookkeeping.  I doubt we can make such an
> assumption for all our uses of readdir(), or that it's okay to
> miss or double-process files in every one of them.
> 
> I'm still of the opinion that the best thing to do is disclaim
> safety of storing a database on NFS.

It would be nice to have some details on the docs about why NFS can
cause problems so we have something to point to when people ask.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Do not let urgent matters crowd out time for investment in the future.

  4. Re: Fwd: Re: A new look at old NFS readdir() problems?

    Larry Rosenman <ler@lerctr.org> — 2025-01-02T21:16:06Z

    On 01/02/2025 2:50 pm, Bruce Momjian wrote:
> On Thu, Jan  2, 2025 at 03:48:53PM -0500, Tom Lane wrote:
>> Larry Rosenman <ler@lerctr.org> writes:
>> > @Tom Lane: This is what Rick Macklem (NFS dev on FreeBSD) has to say on
>> > my issue.
>> 
>> Thanks for reaching out to him.  So if I'm reading this correctly,
>> there's little point in filing a FreeBSD bug because it'll be
>> dismissed as unfixable.
>> 
>> This leaves us in rather a nasty position.  Sure, we could rewrite
>> rmtree() as Thomas suggested upthread, but I'm still of the opinion
>> that that's smearing lipstick on a pig.  rmtree() is the least of
>> our worries: it doesn't need to expect that anybody else will be
>> modifying the target directory, plus it can easily restart its scan
>> without complicated bookkeeping.  I doubt we can make such an
>> assumption for all our uses of readdir(), or that it's okay to
>> miss or double-process files in every one of them.
>> 
>> I'm still of the opinion that the best thing to do is disclaim
>> safety of storing a database on NFS.
> 
> It would be nice to have some details on the docs about why NFS can
> cause problems so we have something to point to when people ask.

What about doing what Rick suggests?
do {
       dir = opendir("X");
        dp = readdir(dir);
        if (dp != NULL)
             unlink(dp->d_name);
         close(dir);
} while (dp != NULL);
?
-- 
Larry Rosenman                     http://www.lerctr.org/~ler
Phone: +1 214-642-9640                 E-Mail: ler@lerctr.org
US Mail: 13425 Ranch Road 620 N, Apt 718, Austin, TX 78717-1010

  5. Re: Fwd: Re: A new look at old NFS readdir() problems?

    Robert Haas <robertmhaas@gmail.com> — 2025-01-02T21:35:26Z

    On Thu, Jan 2, 2025 at 3:49 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I'm still of the opinion that the best thing to do is disclaim
> safety of storing a database on NFS.

If we're going to disclaim support for NFS, it would certainly be
better to do that clearly and with reasons than otherwise. However, I
suspect a substantial number of users will still use NFS and then
blame us when it doesn't work; or alternatively say that our software
sucks because it doesn't work on NFS. Neither of which are very nice
outcomes.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  6. Re: Fwd: Re: A new look at old NFS readdir() problems?

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-01-02T21:52:34Z

    Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Jan 2, 2025 at 3:49 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I'm still of the opinion that the best thing to do is disclaim
>> safety of storing a database on NFS.

> If we're going to disclaim support for NFS, it would certainly be
> better to do that clearly and with reasons than otherwise. However, I
> suspect a substantial number of users will still use NFS and then
> blame us when it doesn't work; or alternatively say that our software
> sucks because it doesn't work on NFS. Neither of which are very nice
> outcomes.

Are you prepared to buy into "we will make every bit of code that uses
readdir() proof against arbitrary lies from readdir()"?  I'm not:
I cannot see how to do that in any but the simplest cases -- rmtree()
being about the simplest.  Even if we had a bulletproof design in
mind, it's pretty hard to believe that future patches would apply
it every time.  I think it's inevitable that we'd have bugs like
"sometimes not every file gets fsync'd", which would be impossible
to find until someone's data gets eaten, and maybe still impossible
to find afterwards.

(To be clear: if this is how FreeBSD acts, then I'm afraid we already
do have such bugs.  The rmtree case is just easier to observe than a
missed fsync.)

			regards, tom lane

  7. Re: Fwd: Re: A new look at old NFS readdir() problems?

    Larry Rosenman <ler@lerctr.org> — 2025-01-02T22:50:22Z

    On 01/02/2025 3:42 pm, Thomas Munro wrote:
> On Fri, Jan 3, 2025 at 10:16 AM Larry Rosenman <ler@lerctr.org> wrote:
>> What about doing what Rick suggests?
>> do {
>>        dir = opendir("X");
>>         dp = readdir(dir);
>>         if (dp != NULL)
>>              unlink(dp->d_name);
>>          close(dir);
>> } while (dp != NULL);
>> ?
> 
> That only works for unlink loops where we expect no concurrent
> modifications.  DROP DATABASE qualifies, but we use readdir() on
> directories that might be changing in various other places, including
> backups.  They'd silently fail to do their job, and can't use that
> technique as they aren't unlinking as they go and so they would never
> make progress.  Which leads to the lipstick/pig conclusion: we'd have
> other, worse, buggy behaviour still.
> 
> It might be possible to make our own readdir snapshot thing that
> checks a shmem counter of all links/unlinks before and after and
> retries.  But ugh...

Would it make sense for ONLY drop database to have the above loop?

-- 
Larry Rosenman                     http://www.lerctr.org/~ler
Phone: +1 214-642-9640                 E-Mail: ler@lerctr.org
US Mail: 13425 Ranch Road 620 N, Apt 718, Austin, TX 78717-1010

  8. Re: Fwd: Re: A new look at old NFS readdir() problems?

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-01-02T22:56:02Z

    Larry Rosenman <ler@lerctr.org> writes:
> Would it make sense for ONLY drop database to have the above loop?

Not really.  We'd just be papering over the most-easily-observable
consequence of readdir's malfeasance.  There'd still be problems
like basebackups omitting files, missed fsyncs potentially leading
to data loss, etc.

I am sort of wondering though why we've not heard reports of this
many times before.  Did FreeBSD recently change something in this
area?  Also, if as they argue it's a fundamental problem in the NFS
protocol, why are we failing to reproduce it with other clients?

			regards, tom lane

  9. Re: Fwd: Re: A new look at old NFS readdir() problems?

    Larry Rosenman <ler@lerctr.org> — 2025-01-02T22:58:35Z

    On 01/02/2025 4:56 pm, Tom Lane wrote:
> Larry Rosenman <ler@lerctr.org> writes:
>> Would it make sense for ONLY drop database to have the above loop?
> 
> Not really.  We'd just be papering over the most-easily-observable
> consequence of readdir's malfeasance.  There'd still be problems
> like basebackups omitting files, missed fsyncs potentially leading
> to data loss, etc.
> 
> I am sort of wondering though why we've not heard reports of this
> many times before.  Did FreeBSD recently change something in this
> area?  Also, if as they argue it's a fundamental problem in the NFS
> protocol, why are we failing to reproduce it with other clients?
> 
> 			regards, tom lane
Pre version 16, we worked just fine.  There was a change in 16.

-- 
Larry Rosenman                     http://www.lerctr.org/~ler
Phone: +1 214-642-9640                 E-Mail: ler@lerctr.org
US Mail: 13425 Ranch Road 620 N, Apt 718, Austin, TX 78717-1010

  10. Re: Fwd: Re: A new look at old NFS readdir() problems?

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-01-02T23:05:13Z

    Larry Rosenman <ler@lerctr.org> writes:
> On 01/02/2025 4:56 pm, Tom Lane wrote:
>> I am sort of wondering though why we've not heard reports of this
>> many times before.  Did FreeBSD recently change something in this
>> area?  Also, if as they argue it's a fundamental problem in the NFS
>> protocol, why are we failing to reproduce it with other clients?

> Pre version 16, we worked just fine.  There was a change in 16.

Well, as Thomas already mentioned, rmtree() used to work by reading
the directory and making an in-memory copy of the whole file list
before it started to delete anything.  But reverting that wouldn't
fix all of the other problems that we now know exist.  (In other
words, "we worked just fine" only for small values of "fine".)

			regards, tom lane

  11. Re: Fwd: Re: A new look at old NFS readdir() problems?

    Thomas Munro <thomas.munro@gmail.com> — 2025-01-02T23:08:50Z

    On Fri, Jan 3, 2025 at 10:53 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> (To be clear: if this is how FreeBSD acts, then I'm afraid we already
> do have such bugs.  The rmtree case is just easier to observe than a
> missed fsync.)

For what little it's worth, I'm not quite convinced yet that FreeBSD's
client isn't more broken than it needs to be.  Lots of systems I
looked at have stable cookies in practice (as NFS 4 recommends),
including the one used in this report, so it seems like a more basic
problem.  At the risk of being wrong on the internet, I don't see any
fundamental reason why a readdir() scan can't have no-skip,
no-duplicate, no-fail semantics for stable-cookie, no-verification
servers.  And this case works perfectly with a couple of other NFS
clients implementations that you and I tried.

As for systems that don't have stable cookies, well then they should
implement the cookie verification scheme and AFAICS the readdir() scan
should then fail if it can't recover, or it would expose isolation
anomalies offensive to database hacker sensibilities.  I think it
should be theoretically possible to recover in some happy cases
(maybe: when enough state is still around in cache to deduplicate).
But that shouldn't be necessary on filers using eg ZFS or BTRFS whose
cookies are intended to be stable.  A server could also do MVCC magic,
and from a quick google, I guess NetApp WAFL might do that as it has
the concept of "READDIR expired", which smells a bit like ORA-01555:
snapshot too old.

  12. Re: Fwd: Re: A new look at old NFS readdir() problems?

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-01-02T23:20:15Z

    Thomas Munro <thomas.munro@gmail.com> writes:
> For what little it's worth, I'm not quite convinced yet that FreeBSD's
> client isn't more broken than it needs to be.

I'm suspicious of that too.  The wireshark trace you described is hard
to read any other way than that FreeBSD went out of its way to deliver
incorrect information.  I'm prepared to believe that we can't work
correctly on NFS servers that don't do the stable-cookie thing, but
why isn't it succeeding on ones that do?

			regards, tom lane

  13. Re: Fwd: Re: A new look at old NFS readdir() problems?

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-01-03T01:58:01Z

    I wrote:
> Thomas Munro <thomas.munro@gmail.com> writes:
>> For what little it's worth, I'm not quite convinced yet that FreeBSD's
>> client isn't more broken than it needs to be.

> I'm suspicious of that too.

I poked at this a little further.  I made the attached stand-alone
test case (you don't need any more than "cc -o rmtree rmtree.c"
to build it, then point the script at some NFS-mounted directory).
This fails with my NAS at least as far back as FreeBSD 11.0.
I also tried it on NetBSD 9.2 which seems fine.

			regards, tom lane

  14. Re: Fwd: Re: A new look at old NFS readdir() problems?

    Robert Haas <robertmhaas@gmail.com> — 2025-01-03T13:33:02Z

    On Thu, Jan 2, 2025 at 4:52 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Are you prepared to buy into "we will make every bit of code that uses
> readdir() proof against arbitrary lies from readdir()"?  I'm not:
> I cannot see how to do that in any but the simplest cases -- rmtree()
> being about the simplest.  Even if we had a bulletproof design in
> mind, it's pretty hard to believe that future patches would apply
> it every time.  I think it's inevitable that we'd have bugs like
> "sometimes not every file gets fsync'd", which would be impossible
> to find until someone's data gets eaten, and maybe still impossible
> to find afterwards.

No. We obviously cannot guarantee correct output in the face of
arbitrarily incorrect input. What I'm doing is being practical about
how the real world works. People use NFS. This is not that different
from the fsync debacle a few years ago -- the fsync semantics on Linux
did (and AFAIK, still do) make it impossible to write correct
programs, and then on top of that our code had bugs in it, too. We
didn't just say "oh, well, I guess you shouldn't use Linux". We tried
to make our code as robust as it could be in the face of kernel code
that behaved in a manner that was fairly ridiculous relative to our
needs. This case doesn't seem that different to me.

I certainly think it's reasonable to debate whether any particular
mitigations that someone might propose adding to our code are
unreasonably difficult. But the complaint as presented is "the code
you used to have worked better than the code you have now," which
doesn't necessarily support the idea that the requested changes are
unreasonable. Of course, if FreeBSD is buggier than it needs to be,
then I hope that also gets fixed. But even if it doesn't, surely the
right thing to do would be to clearly document that we don't work
*with NFS on FreeBSD* rather than that we don't work with *any NFS*.
Unless of course we really (and unavoidably) don't work with any NFS,
and then we should document that, with reasons.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  15. Re: Fwd: Re: A new look at old NFS readdir() problems?

    Greg Sabino Mullane <htamfids@gmail.com> — 2025-01-03T14:47:09Z

    On Fri, Jan 3, 2025 at 8:33 AM Robert Haas <robertmhaas@gmail.com> wrote:

> We tried to make our code as robust as it could be in the face of kernel
> code that behaved in a manner that was fairly ridiculous relative to our
> needs. This case doesn't seem that different to me.
>

+1. Seems a shame that freebsd chooses such "optimizations", but making our
code do various workarounds and jump through hoops to support various OS
quirks (hello Win32 fans!) seems a burden we agreed to take on a long time
ago.

Cheers,
Greg

  16. Re: Fwd: Re: A new look at old NFS readdir() problems?

    David Steele <david@pgbackrest.org> — 2025-01-03T16:48:15Z

    On 1/3/25 09:47, Greg Sabino Mullane wrote:
> On Fri, Jan 3, 2025 at 8:33 AM Robert Haas <robertmhaas@gmail.com 
> <mailto:robertmhaas@gmail.com>> wrote:
> 
>     We tried to make our code as robust as it could be in the face of
>     kernel code that behaved in a manner that was fairly ridiculous
>     relative to our needs. This case doesn't seem that different to me.
> 
> 
> +1. Seems a shame that freebsd chooses such "optimizations", but making 
> our code do various workarounds and jump through hoops to support 
> various OS quirks (hello Win32 fans!) seems a burden we agreed to take 
> on a long time ago.

FWIW, we observed this issue in pgBackRest a few years ago -- as you can 
imagine we do a lot of scanning so readdir gets a real workout.

We had one issue reported [1] involving Alpine Linux and CIFS and 
another [2] with SLES and NFS. We also had at least one internal report 
that involved RHEL and a proprietary storage appliance. I'm not certain 
if we received any reports for FreeBSD but it kind of rings a bell.

Over some time and various reports it seemed that any storage was 
potentially a problem. I resisted the notion that we would have to work 
around something that seemed to be an obvious kernel bug but in the end 
I capitulated.

We fixed this by making a snapshot of each directory before performing 
any operations on that directory (as has been suggested upthread). One 
advantage we have is that our storage is very centralized since we deal 
with a number of storage types so there are no readdirs in the general 
code base. It was still a pretty major patch [3] but a lot of it was 
removing the callbacks that we had used previously and adding 
optimizations to reduce memory consumption.

One more thing to note -- we are still assuming that Postgres is running 
on storage that is not subject to this issue. Even with our new 
methodology if Postgres is deleting files while we are trying to build a 
backup manifest that could cause us (and base backup) problems. The only 
solution I came up with for that problem was to keep reading the 
directory until we get two snapshots that match -- not very attractive 
but probably workable for pgBackRest. I doubt the same could be said for 
Postgres.

Regards,
-David

---

[1] https://github.com/pgbackrest/pgbackrest/issues/1754
[2] https://github.com/pgbackrest/pgbackrest/issues/1423
[3] https://github.com/pgbackrest/pgbackrest/commit/75623d45

  17. Re: Fwd: Re: A new look at old NFS readdir() problems?

    Peter Eisentraut <peter@eisentraut.org> — 2025-01-03T17:31:37Z

    On 03.01.25 02:58, Tom Lane wrote:
> I wrote:
>> Thomas Munro <thomas.munro@gmail.com> writes:
>>> For what little it's worth, I'm not quite convinced yet that FreeBSD's
>>> client isn't more broken than it needs to be.
> 
>> I'm suspicious of that too.
> 
> I poked at this a little further.  I made the attached stand-alone
> test case (you don't need any more than "cc -o rmtree rmtree.c"
> to build it, then point the script at some NFS-mounted directory).
> This fails with my NAS at least as far back as FreeBSD 11.0.
> I also tried it on NetBSD 9.2 which seems fine.

If you use some GUI file manager, point at a directory, and select 
"remove this directory with everything in it", what does it do 
internally?  Surely it runs a readdir() loop and unlinks the files as it 
gets to them?  Or does it indeed slurp the whole directory tree into 
memory before starting the deletion?  It seems like we're probably not 
the first to have this use case, so there should be some information or 
problem reports about this somewhere out there.  Or if not, then I'd be 
tempted to think, someone broke it recently, even if the NFS hacker 
argues that it cannot possibly work and never should have worked.

  18. Re: Fwd: Re: A new look at old NFS readdir() problems?

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-01-03T18:12:43Z

    Peter Eisentraut <peter@eisentraut.org> writes:
> On 03.01.25 02:58, Tom Lane wrote:
>> I poked at this a little further.  I made the attached stand-alone
>> test case (you don't need any more than "cc -o rmtree rmtree.c"
>> to build it, then point the script at some NFS-mounted directory).
>> This fails with my NAS at least as far back as FreeBSD 11.0.
>> I also tried it on NetBSD 9.2 which seems fine.

> If you use some GUI file manager, point at a directory, and select 
> "remove this directory with everything in it", what does it do 
> internally?  Surely it runs a readdir() loop and unlinks the files as it 
> gets to them?  Or does it indeed slurp the whole directory tree into 
> memory before starting the deletion?

One thing I noticed while testing yesterday is that "rm -rf foo"
worked even in cases where "rmtree foo" didn't.  I did not look
into FreeBSD's rm code, but I'll bet it has the sort of retry
logic that was recommended to us upthread.  I agree with your
point though that it's hard to believe that everyone does that
in every case where it would matter.

As for pre-existing bug reports, I found

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=57696

but there's basically no new information there, other than
the not-so-surprising fact that renaming directory entries
triggers the failure as efficiently as unlinking does.

			regards, tom lane

  19. Re: Fwd: Re: A new look at old NFS readdir() problems?

    Thomas Munro <thomas.munro@gmail.com> — 2025-01-03T23:39:38Z

    On Sat, Jan 4, 2025 at 7:12 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> One thing I noticed while testing yesterday is that "rm -rf foo"
> worked even in cases where "rmtree foo" didn't.  I did not look
> into FreeBSD's rm code, but I'll bet it has the sort of retry
> logic that was recommended to us upthread.

It punts the directory scan to the fts_read() etc functions (directory
traversal helpers widely inherited from ancestral BSD but not in any
standard), which buffers the scan and probably hides this issue:

https://github.com/freebsd/freebsd-src/blob/main/bin/rm/rm.c
https://github.com/freebsd/freebsd-src/blob/main/lib/libc/gen/fts.c

I doubt that hides all potential problems though, if I have understood
the vague outline of this bug correctly: perhaps if you ran large
enough rm -r, and you unlinked a file concurrently with that loop, you
could break it, that is, cause it to skip innocent files other than
the one you unlinked?

glibc also has an implementation of the BSD fts functions, and uses
them in its rm:

https://github.com/coreutils/coreutils/blob/master/src/rm.c
https://github.com/coreutils/coreutils/blob/master/src/remove.c

> As for pre-existing bug reports, I found
>
> https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=57696
>
> but there's basically no new information there, other than
> the not-so-surprising fact that renaming directory entries
> triggers the failure as efficiently as unlinking does.

FWIW I am discussing this off-list with Rick, I *think* we can
distinguish between "gripes abouts NFS that we can't do much about"
and "extra thing going wrong here".  The fog of superstition around
NFS is thick because so many investigations end at a vendor
boundary/black box, but here we can understand the cookie scheme and
trace through all the layers here...

  20. Re: Fwd: Re: A new look at old NFS readdir() problems?

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-01-04T01:01:12Z

    Thomas Munro <thomas.munro@gmail.com> writes:
> I doubt that hides all potential problems though, if I have understood
> the vague outline of this bug correctly: perhaps if you ran large
> enough rm -r, and you unlinked a file concurrently with that loop, you
> could break it, that is, cause it to skip innocent files other than
> the one you unlinked?

Yeah.  The thing that makes this untenably bad for us is that it's not
only your own process's actions that can break readdir(), it's the
actions of some unrelated process.  So even "buffer the whole
directory contents" isn't a reliable fix, because someone else could
unlink a file while you're reading the directory.

One way to "fix" this from our side is to institute some system-wide
interlock that prevents file deletions (and renames, and maybe even
creations?) while any process is executing a readdir loop.  That's
several steps beyond awful from a performance standpoint, and what's
worse is that it's still not reliable.  It only takes one bit of
code that isn't on board with the locking protocol to put you right
back at square one.  It might not even be code that's part of Postgres
proper, so long as it has access to PGDATA.  As an example: it'd be
unsafe to modify postgresql.conf with emacs, or any other editor that
likes to make a backup file.  So IMV that's no fix at all.

The only other thing I can think of is to read and buffer the whole
directory (no matter how large), and then do it *again*, and repeat
till we get the same results twice in a row.  That's likewise just
horrid from a performance standpoint.  Worse, I'm not entirely
convinced that it's a 100% fix: it seems to rely on rather debatable
assumptions about the possible consequences of concurrent directory
mods.

> FWIW I am discussing this off-list with Rick, I *think* we can
> distinguish between "gripes abouts NFS that we can't do much about"
> and "extra thing going wrong here".  The fog of superstition around
> NFS is thick because so many investigations end at a vendor
> boundary/black box, but here we can understand the cookie scheme and
> trace through all the layers here...

I wouldn't have any problem with saying that we don't support NFS
implementations that don't have stable cookies.  But so far I haven't
found any supported platform except FreeBSD that fails the rmtree test
against my Synology NAS.  I think the circumstantial evidence that
FreeBSD is doing something wrong, or wronger than necessary, is
strong.

			regards, tom lane

  21. Re: Fwd: Re: A new look at old NFS readdir() problems?

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-01-04T08:15:51Z

    I wrote:
> I wouldn't have any problem with saying that we don't support NFS
> implementations that don't have stable cookies.  But so far I haven't
> found any supported platform except FreeBSD that fails the rmtree test
> against my Synology NAS.

To expand on that: I've now found that Linux, macOS, NetBSD, OpenBSD,
and illumos/OpenIndiana[1] pass that test.  I don't believe that Windows
does NFS.  That means that FreeBSD is the only one of our supported
platforms that fails.  I think we should simply document that NFS on
FreeBSD is broken, and await somebody getting annoyed enough to
fix that brokenness.

			regards, tom lane

[1] Getting OpenIndiana to work under qemu is not a task for those
easily discouraged.

  22. Re: Fwd: Re: A new look at old NFS readdir() problems?

    Thomas Munro <thomas.munro@gmail.com> — 2025-01-04T16:07:18Z

    On Sat, Jan 4, 2025 at 5:48 AM David Steele <david@pgbackrest.org> wrote:
> We had one issue reported [1] involving Alpine Linux and CIFS and

Not directly relevant for pgbackrest probably, but I noticed that
Alpine comes up in a few reports of failing rm -r on CIFS.  I think it
might be because BSD and GNU rm use fts to buffer pathnames in user
space (narrow window), while Alpine uses busybox rm which has a
classic readdir()/unlink() loop:

https://github.com/brgl/busybox/blob/master/coreutils/rm.c
https://github.com/brgl/busybox/blob/master/libbb/remove_file.c

As for CIFS, there are lots of reports of this sort of thing from
Linux CIFS clients.  I am suspicious of the 32 bit monotonic
resume_key apparently being used to seek to a starting position.  I
don't plan to investigate myself, but ... is that even trying to avoid
skips and duplicates?

  23. Re: Fwd: Re: A new look at old NFS readdir() problems?

    Robert Haas <robertmhaas@gmail.com> — 2025-01-06T14:20:18Z

    On Sat, Jan 4, 2025 at 3:15 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> To expand on that: I've now found that Linux, macOS, NetBSD, OpenBSD,
> and illumos/OpenIndiana[1] pass that test.  I don't believe that Windows
> does NFS.  That means that FreeBSD is the only one of our supported
> platforms that fails.  I think we should simply document that NFS on
> FreeBSD is broken, and await somebody getting annoyed enough to
> fix that brokenness.

Yeah, that seems like very strong evidence against FreeBSD, but I
think Thomas Munro's point about CIFS is worth considering. That is
rather widely used, and if the same workarounds would help both that
and FreeBSD's NFS, we might want to adopt it even if it's not a
complete fix.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  24. Re: Fwd: Re: A new look at old NFS readdir() problems?

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-01-06T14:49:22Z

    Robert Haas <robertmhaas@gmail.com> writes:
> Yeah, that seems like very strong evidence against FreeBSD, but I
> think Thomas Munro's point about CIFS is worth considering. That is
> rather widely used, and if the same workarounds would help both that
> and FreeBSD's NFS, we might want to adopt it even if it's not a
> complete fix.

TBH, I am happy that PG is now failing in a fairly visible way on
filesystems that are broken in this fashion.  I think that is better
than silent data corruption in obscure circumstances, which is where
we were before and would be again if we band-aid rmtree() and do
nothing else.  Nor do I think it's worth the effort to try to become
fully bulletproof on the point.

I think we should document that CIFS is unsupported.  The docs
could say something like:

	Storing databases on filesystems with unreliable readdir() is
	not supported and can lead to data corruption.  If you observe
	warnings like "directory is not empty" when trying to drop a
	database, that is strong evidence that the filesystem has
	unreliable readdir().  Filesystems known to have this problem
	include NFS on FreeBSD and CIFS on all platforms.

We could s/FreeBSD/older FreeBSD/ if something comes of Thomas'
efforts to fix that situation.

			regards, tom lane

  25. Re: Fwd: Re: A new look at old NFS readdir() problems?

    David Steele <david@pgbackrest.org> — 2025-01-06T16:41:20Z

    On 1/4/25 11:07, Thomas Munro wrote:
> On Sat, Jan 4, 2025 at 5:48 AM David Steele <david@pgbackrest.org> wrote:
>> We had one issue reported [1] involving Alpine Linux and CIFS and
> 
> Not directly relevant for pgbackrest probably, but I noticed that
> Alpine comes up in a few reports of failing rm -r on CIFS.  I think it
> might be because BSD and GNU rm use fts to buffer pathnames in user
> space (narrow window), while Alpine uses busybox rm which has a
> classic readdir()/unlink() loop:

Yeah, this doesn't affect pgBackRest because we have our own rmtree that 
uses snapshots (for the last few years, at least).

> As for CIFS, there are lots of reports of this sort of thing from
> Linux CIFS clients.

There may be users running Postgres on CIFS but my guess is that is rare 
-- at least I have never seen anyone doing it.

I'm more concerned about the report we saw on SUSE/NFS [1]. If that 
report is accurate it indicates this may not be something we can just 
document and move on from -- unless we are willing to entirely drop 
support for NFS.

Regards,
-David

[1] https://github.com/pgbackrest/pgbackrest/issues/1423

  26. Re: Fwd: Re: A new look at old NFS readdir() problems?

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-01-07T00:08:55Z

    David Steele <david@pgbackrest.org> writes:
> On 1/4/25 11:07, Thomas Munro wrote:
>> As for CIFS, there are lots of reports of this sort of thing from
>> Linux CIFS clients.

> There may be users running Postgres on CIFS but my guess is that is rare 
> -- at least I have never seen anyone doing it.

It'd be news to me too.  I wondered if I could test it locally, but
while my NAS knows half a dozen such protocols it's never heard of
CIFS.

> I'm more concerned about the report we saw on SUSE/NFS [1]. If that 
> report is accurate it indicates this may not be something we can just 
> document and move on from -- unless we are willing to entirely drop 
> support for NFS.
> [1] https://github.com/pgbackrest/pgbackrest/issues/1423

I installed an up-to-date OpenSUSE image (Leap 15.6) and it passes
my "rmtree" test just fine with my NAS.  The report you cite
doesn't have any details on what the NFS server was, but I'd be
inclined to guess that that server's filesystem lacked support
for stable NFS cookies.

			regards, tom lane

  27. Re: Fwd: Re: A new look at old NFS readdir() problems?

    David Steele <david@pgbackrest.org> — 2025-01-07T01:03:05Z

    On 1/6/25 19:08, Tom Lane wrote:
> David Steele <david@pgbackrest.org> writes:
>> On 1/4/25 11:07, Thomas Munro wrote:
>>> As for CIFS, there are lots of reports of this sort of thing from
>>> Linux CIFS clients.
> 
>> There may be users running Postgres on CIFS but my guess is that is rare
>> -- at least I have never seen anyone doing it.
> 
> It'd be news to me too.  I wondered if I could test it locally, but
> while my NAS knows half a dozen such protocols it's never heard of
> CIFS.

You may also know it as SMB or Samba. pgBackRest skips directory fsyncs 
if the repository type is set to CIFS so I think we'd know if anybody 
was running on CIFS as I'm fairly certain a directory fsync will return 
a hard error. That may be implementation dependent, though.

>> I'm more concerned about the report we saw on SUSE/NFS [1]. If that
>> report is accurate it indicates this may not be something we can just
>> document and move on from -- unless we are willing to entirely drop
>> support for NFS.
>> [1] https://github.com/pgbackrest/pgbackrest/issues/1423
> 
> I installed an up-to-date OpenSUSE image (Leap 15.6) and it passes
> my "rmtree" test just fine with my NAS.  The report you cite
> doesn't have any details on what the NFS server was, but I'd be
> inclined to guess that that server's filesystem lacked support
> for stable NFS cookies.

The internal report we received might have had a similar cause. Sure 
seems like a minefield for any user trying to figure out if their setup 
is compliant, though. In many setups (especially production) a drop 
database is rare.

Regards,
-David

  28. Re: Fwd: Re: A new look at old NFS readdir() problems?

    Bruce Momjian <bruce@momjian.us> — 2025-01-08T17:40:00Z

    On Tue, Jan  7, 2025 at 01:03:05AM +0000, David Steele wrote:
> > > I'm more concerned about the report we saw on SUSE/NFS [1]. If that
> > > report is accurate it indicates this may not be something we can just
> > > document and move on from -- unless we are willing to entirely drop
> > > support for NFS.
> > > [1] https://github.com/pgbackrest/pgbackrest/issues/1423
> > 
> > I installed an up-to-date OpenSUSE image (Leap 15.6) and it passes
> > my "rmtree" test just fine with my NAS.  The report you cite
> > doesn't have any details on what the NFS server was, but I'd be
> > inclined to guess that that server's filesystem lacked support
> > for stable NFS cookies.
> 
> The internal report we received might have had a similar cause. Sure seems
> like a minefield for any user trying to figure out if their setup is
> compliant, though. In many setups (especially production) a drop database is
> rare.

Will people now always get a clear error on failure?  Crazy idea, but
could we have initdb or postmaster start test this? 

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Do not let urgent matters crowd out time for investment in the future.

  29. Re: Fwd: Re: A new look at old NFS readdir() problems?

    David Steele <david@pgbackrest.org> — 2025-01-08T19:18:16Z

    On 1/8/25 12:40, Bruce Momjian wrote:
> On Tue, Jan  7, 2025 at 01:03:05AM +0000, David Steele wrote:
>>>> I'm more concerned about the report we saw on SUSE/NFS [1]. If that
>>>> report is accurate it indicates this may not be something we can just
>>>> document and move on from -- unless we are willing to entirely drop
>>>> support for NFS.
>>>> [1] https://github.com/pgbackrest/pgbackrest/issues/1423
>>>
>>> I installed an up-to-date OpenSUSE image (Leap 15.6) and it passes
>>> my "rmtree" test just fine with my NAS.  The report you cite
>>> doesn't have any details on what the NFS server was, but I'd be
>>> inclined to guess that that server's filesystem lacked support
>>> for stable NFS cookies.
>>
>> The internal report we received might have had a similar cause. Sure seems
>> like a minefield for any user trying to figure out if their setup is
>> compliant, though. In many setups (especially production) a drop database is
>> rare.
> 
> Will people now always get a clear error on failure?  

The error will be something like "directory is not empty" when trying to 
drop a database. So not very clear at all.

> Crazy idea, but
> could we have initdb or postmaster start test this?

I think this test would be too expensive for postmaster start as it 
would require creating and deleting a large number of files (maybe 
multiple times). I suppose it could be a special setting but I doubt 
that would be popular.

initdb might be more promising but clusters are frequently 
copied/restored to different storage so I'm not sure if it would be too 
helpful in the long run.

Regards,
-David

  30. Re: Fwd: Re: A new look at old NFS readdir() problems?

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-01-08T20:21:27Z

    Bruce Momjian <bruce@momjian.us> writes:
> Will people now always get a clear error on failure?  Crazy idea, but
> could we have initdb or postmaster start test this? 

It'd require adding quite a lot of cycles: I'd guess you'd need to
create/drop a hundred or a thousand files to be sure of seeing the
problem.

Another problem is that even if the primary data directory is okay,
a tablespace could be created on a filesystem that's not okay.

I doubt that it's worth it.

			regards, tom lane

  31. Re: Fwd: Re: A new look at old NFS readdir() problems?

    Bruce Momjian <bruce@momjian.us> — 2025-01-08T20:23:02Z

    On Wed, Jan  8, 2025 at 03:21:27PM -0500, Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > Will people now always get a clear error on failure?  Crazy idea, but
> > could we have initdb or postmaster start test this? 
> 
> It'd require adding quite a lot of cycles: I'd guess you'd need to
> create/drop a hundred or a thousand files to be sure of seeing the
> problem.
> 
> Another problem is that even if the primary data directory is okay,
> a tablespace could be created on a filesystem that's not okay.
> 
> I doubt that it's worth it.

Oh, I thought if it could be recreated using just two files, it would be
worth it --- obviously not.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Do not let urgent matters crowd out time for investment in the future.

  32. Re: Fwd: Re: A new look at old NFS readdir() problems?

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-01-08T20:41:45Z

    David Steele <david@pgbackrest.org> writes:
> On 1/8/25 12:40, Bruce Momjian wrote:
>> Will people now always get a clear error on failure?  

> The error will be something like "directory is not empty" when trying to 
> drop a database. So not very clear at all.

Conceivably we could adjust rmtree() to print a HINT mentioning that
unsafe network storage is a possible cause of this error.  But I'm
a little worried that that'd be crying wolf, because on most setups
a much more likely explanation is somebody concurrently creating a
file in the targeted directory.

			regards, tom lane