From: | Thomas Munro <thomas(dot)munro(at)gmail(dot)com> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: allow_in_place_tablespaces vs. pg_basebackup |
Date: | 2023-03-28 04:15:25 |
Message-ID: | CA+hUKGJhLNV_AKp9fZ=H_Cr5SBkMS-Ynhd2Ro=EHCSobj4Z9NA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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.
Attachment | Content-Type | Size |
---|---|---|
0001-Fix-pg_basebackup-with-in-place-tablespaces-some-mor.patch | text/x-patch | 11.7 KB |
0002-fixup.patch | text/x-patch | 2.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Hayato Kuroda (Fujitsu) | 2023-03-28 04:19:04 | RE: PGdoc: add missing ID attribute to create_subscription.sgml |
Previous Message | houzj.fnst@fujitsu.com | 2023-03-28 04:12:34 | RE: Support logical replication of DDLs |