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
- 0001-Fix-pg_basebackup-with-in-place-tablespaces-some-mor.patch (text/x-patch) patch 0001
- 0002-fixup.patch (text/x-patch) patch 0002
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
-
Fix pg_basebackup with in-place tablespaces some more.
- 363e8f911546 16.0 landed
-
Fix pg_basebackup with in-place tablespaces.
- c6f2f01611d4 15.0 cited
-
Allow "in place" tablespaces.
- 7170f2159fb2 15.0 cited
-
Avoid including tablespaces inside PGDATA twice in base backups
- b168c5ef2730 9.4.0 cited
-
Tablespaces. Alternate database locations are dead, long live tablespaces.
- 2467394ee156 8.0.0 cited