Re: allow_in_place_tablespaces vs. pg_basebackup

Thomas Munro <thomas.munro@gmail.com>

From: Thomas Munro <thomas.munro@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, pgsql-hackers@postgresql.org
Date: 2023-03-28T04:15:25Z
Lists: pgsql-hackers

Attachments

The commit message explains prettty well, and it seems to work in
simple testing, and yeah commit c6f2f016 was not a work of art.
pg_basebackeup --format=plain "worked", but your way is better.  I
guess we should add a test of -Fp too, to keep it working?  Here's one
of those.

I know it's not your patch's fault, but I wonder if this might break
something.  We have this strange beast ti->rpath (commit b168c5ef in
2014):

+                               /*
+                                * Relpath holds the relative path of
the tablespace directory
+                                * when it's located within PGDATA, or
NULL if it's located
+                                * elsewhere.
+                                */

That's pretty confusing, because relative paths have been banned since
the birth of tablespaces (commit 2467394ee15, 2004):

+       /*
+        * Allowing relative paths seems risky
+        *
+        * this also helps us ensure that location is not empty or whitespace
+        */
+       if (!is_absolute_path(location))
+               ereport(ERROR,
+                               (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                                errmsg("tablespace location must be
an absolute path")));

The discussion that produced the contradiction:
https://www.postgresql.org/message-id/flat/m2ob3vl3et.fsf%402ndQuadrant.fr

I guess what I'm wondering here is if there is a hazard where we
confuse these outlawed tablespaces that supposedly roam the plains
somewhere in your code that is assuming that "relative implies
in-place".  Or if not now, maybe in future changes.  I wonder if these
"semi-supported-but-don't-tell-anyone" relative symlinks are worthy of
a defensive test (or is it in there somewhere already?).

Originally when trying to implement allow_in_place_tablespaces, I
instead tried allowing limited relative tablespaces, so you could use
LOCATION 'pg_tblspc/my_directory', and then it would still create a
symlink 1234 -> my_directory, which probably would have All Just
Worked™ given the existing ti->rpath stuff, and possibly made the
users that thread was about happy, but I had to give that up because
it didn't work on Windows (no relative symlinks).  Oh well.

Commits

  1. Fix pg_basebackup with in-place tablespaces some more.

  2. Fix pg_basebackup with in-place tablespaces.

  3. Allow "in place" tablespaces.

  4. Avoid including tablespaces inside PGDATA twice in base backups

  5. Tablespaces. Alternate database locations are dead, long live tablespaces.