Re: Possible race condition in pg_mkdir_p()?

From: Ning Yu <nyu(at)pivotal(dot)io>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Paul Guo <pguo(at)pivotal(dot)io>
Subject: Re: Possible race condition in pg_mkdir_p()?
Date: 2019-07-31 04:26:30
Message-ID: CAKmaiL12mswvwkh6msfuFmeSMQ8R7Geo0aB6uHTG9jw-THHCag@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Jul 31, 2019 at 12:04 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Tue, Jul 30, 2019 at 06:22:59PM +0800, Ning Yu wrote:
> > In fact personally I'm thinking that whether could we replace all uses of
> > MakePGDirectory() with pg_mkdir_p(), so we could simplify
> > TablespaceCreateDbspace() and PathNameCreateTemporaryDir() and other callers
> > significantly.
>
> I would still keep the wrapper, but I think that as a final result we
> should be able to get the code in PathNameCreateTemporaryDir() shaped
> in such a way that there are no multiple attempts at calling
> MakePGDirectory() on EEXIST. This has been introduced by dc6c4c9d to
> allow sharing temporary files between backends, which is rather recent
> but a fixed set of two retries is not a deterministic method of
> resolution.
>
> > Well, we should have fixed problem 2, this is our major purpose of the patch
> > 0001, it performs sanity check with stat() after mkdir() at each part of the
> > path.
>
> I just reuse the script presented at the top of the thread with n=2,
> and I get that:
> pgdata/logs/1.log:creating directory
> pgdata/a/b/c/d/e/f/g/h/i/j/k/l/m/n/o/p/q/r/s/t/u/v/w/x/y/z/1
> ... initdb: error: could not create directory "pgdata/a": File exists

Could I double confirm with you that you made a clean rebuild after
applying the patches? pg_mkdir_p() is compiled as part of libpgport.a,
and the postgres makefile will not relink the initdb binary
automatically, for myself I must 'make clean' and 'make' to ensure
initdb gets relinked.

>
> But the result expected is that all the paths should be created with
> no complains about the parents existing, no? This reproduces on my
> Debian box 100% of the time, for different sub-paths. So something
> looks wrong in your solution. The code comes originally from FreeBSD,
> how do things happen there. Do we get failures if doing something
> like that? I would expect this sequence to not fail:
> for i in `seq 1 100`; do mkdir -p b/c/d/f/g/h/j/$i; done
> --
> Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2019-07-31 04:30:46 Re: SegFault on 9.6.14
Previous Message Michael Paquier 2019-07-31 04:20:01 Re: tap tests driving the database via psql