Re: pg_tablespace_location() failure with allow_in_place_tablespaces - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: pg_tablespace_location() failure with allow_in_place_tablespaces
Date
Msg-id YkUndEFYAdBIFyHu@paquier.xyz
Whole thread Raw
In response to Re: pg_tablespace_location() failure with allow_in_place_tablespaces  (Thomas Munro <thomas.munro@gmail.com>)
Responses Re: pg_tablespace_location() failure with allow_in_place_tablespaces
List pgsql-hackers
On Wed, Mar 30, 2022 at 08:23:25PM +1300, Thomas Munro wrote:
> That leads to the attached patches, the first of which I'd want to back-patch.

Makes sense.

-   if ((fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0)
-       d->ret.d_type = DT_DIR;
    /* For reparse points dwReserved0 field will contain the ReparseTag */
-   else if ((fd.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0 &&
-            (fd.dwReserved0 == IO_REPARSE_TAG_MOUNT_POINT))
+   if ((fd.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0 &&
+       (fd.dwReserved0 == IO_REPARSE_TAG_MOUNT_POINT))
        d->ret.d_type = DT_LNK;
+   else if ((fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0)
+       d->ret.d_type = DT_DIR;

This should also work for plain files, so that looks fine to me.

> Unfortunately while testing this I realised there is something else
> wrong here: if you take a basebackup using tar format, in-place
> tablespaces are skipped (they should get their own OID.tar file, but
> they don't, also no error).  While it wasn't one of my original goals
> for in-place tablespaces to work in every way (and I'm certain some
> external tools would be confused by them), it seems we're pretty close
> so we should probably figure out that piece of the puzzle.  It may be
> obvious why but I didn't have time to dig into that today... perhaps
> instead of just skipping the readlink() we should be writing something
> different into the mapfile and then restoring as appropriate...

Yeah, I saw that in-place tablespaces were part of the main tarball in
base backups as we rely on the existence of a link to decide if the
contents of a path should be separated in a different tarball or not.
This does not strike me as a huge problem in itself, TBH, as the
improvement would be limited to make sure that the base backups could
be correctly restored with multiple tablespaces.  And you can get
pretty much the same amount of coverage to make sure that the backup
contents are correct without fully restoring them.
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: Handle infinite recursion in logical replication setup
Next
From: Andres Freund
Date:
Subject: Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations