Thread
Commits
-
Teach pg_upgrade to handle in-place tablespaces.
- 412036c22d6a 19 (unreleased) landed
-
teach pg_upgrade to handle in-place tablespaces
Nathan Bossart <nathandbossart@gmail.com> — 2025-04-28T21:07:16Z
(Creating new thread from https://postgr.es/m/Z-MaPREQvH5YB0af%40nathan.) On Tue, Mar 25, 2025 at 04:03:57PM -0500, Nathan Bossart wrote: > I also wanted to draw attention to this note in 0003: > > /* > * XXX: The below line is a hack to deal with the fact that we > * presently don't have an easy way to find the corresponding new > * tablespace's path. This will need to be fixed if/when we add > * pg_upgrade support for in-place tablespaces. > */ > new_tablespace = old_tablespace; > > I intend to address this in v19, primarily to enable same-version > pg_upgrade testing with non-default tablespaces. My current thinking is > that we should have pg_upgrade also gather the new cluster tablespace > information and map them to the corresponding tablespaces on the old > cluster. This might require some refactoring in pg_upgrade. In any case, > I didn't feel this should block the feature for v18. Patch attached. -- nathan
-
Re: teach pg_upgrade to handle in-place tablespaces
Nathan Bossart <nathandbossart@gmail.com> — 2025-04-28T22:01:36Z
On Mon, Apr 28, 2025 at 04:07:16PM -0500, Nathan Bossart wrote: > On Tue, Mar 25, 2025 at 04:03:57PM -0500, Nathan Bossart wrote: >> I also wanted to draw attention to this note in 0003: >> >> /* >> * XXX: The below line is a hack to deal with the fact that we >> * presently don't have an easy way to find the corresponding new >> * tablespace's path. This will need to be fixed if/when we add >> * pg_upgrade support for in-place tablespaces. >> */ >> new_tablespace = old_tablespace; >> >> I intend to address this in v19, primarily to enable same-version >> pg_upgrade testing with non-default tablespaces. My current thinking is >> that we should have pg_upgrade also gather the new cluster tablespace >> information and map them to the corresponding tablespaces on the old >> cluster. This might require some refactoring in pg_upgrade. In any case, >> I didn't feel this should block the feature for v18. > > Patch attached. And here is a new version of the patch that should hopefully build on Windows. -- nathan
-
Re: teach pg_upgrade to handle in-place tablespaces
Nathan Bossart <nathandbossart@gmail.com> — 2025-07-01T20:06:04Z
rebased -- nathan
-
Re: teach pg_upgrade to handle in-place tablespaces
Corey Huinker <corey.huinker@gmail.com> — 2025-07-22T00:16:12Z
On Tue, Jul 1, 2025 at 4:06 PM Nathan Bossart <nathandbossart@gmail.com> wrote: > rebased > > -- > nathan Everything here makes sense to me, but I do have one question: In src/bin/pg_upgrade/info.c @@ -616,11 +630,21 @@ process_rel_infos(DbInfo *dbinfo, PGresult *res, void *arg) + if (inplace) + tablespace = psprintf("%s/%s", + os_info.running_cluster->pgdata, + PQgetvalue(res, relnum, i_spclocation)); + else + tablespace = PQgetvalue(res, relnum, i_spclocation); I'm sure it's no big deal, but we've already PQgetvalue() fetched that once for spcloc, and we're going to fetch it again no matter what the value of inplace is. Is there a reason to not reuse spcloc? -
Re: teach pg_upgrade to handle in-place tablespaces
Nathan Bossart <nathandbossart@gmail.com> — 2025-07-22T00:57:32Z
On Mon, Jul 21, 2025 at 08:16:12PM -0400, Corey Huinker wrote: > Everything here makes sense to me, but I do have one question: Thanks for reviewing. > In src/bin/pg_upgrade/info.c > @@ -616,11 +630,21 @@ process_rel_infos(DbInfo *dbinfo, PGresult *res, void > *arg) > + if (inplace) > + tablespace = psprintf("%s/%s", > + os_info.running_cluster->pgdata, > + PQgetvalue(res, relnum, i_spclocation)); > + else > + tablespace = PQgetvalue(res, relnum, i_spclocation); > > I'm sure it's no big deal, but we've already PQgetvalue() fetched that once > for spcloc, and we're going to fetch it again no matter what the value of > inplace is. Is there a reason to not reuse spcloc? I can't think of any reason. Fixed in v4. -- nathan -
Re: teach pg_upgrade to handle in-place tablespaces
Michael Paquier <michael@paquier.xyz> — 2025-07-22T01:37:05Z
On Mon, Jul 21, 2025 at 07:57:32PM -0500, Nathan Bossart wrote: + if (!defined($ENV{oldinstall})) + { + $new->append_conf('postgresql.conf', + "allow_in_place_tablespaces = true"); + $old->append_conf('postgresql.conf', + "allow_in_place_tablespaces = true"); + } This would not choke as long as the old cluster is at least at v10, but well why not. - # For cross-version tests, we can also check that pg_upgrade handles - # tablespaces. + my $tblspc = ''; if (defined($ENV{oldinstall})) { - my $tblspc = PostgreSQL::Test::Utils::tempdir_short(); - $old->safe_psql('postgres', - "CREATE TABLESPACE test_tblspc LOCATION '$tblspc'"); - $old->safe_psql('postgres', - "CREATE DATABASE testdb2 TABLESPACE test_tblspc"); - $old->safe_psql('postgres', - "CREATE TABLE test3 TABLESPACE test_tblspc AS SELECT generate_series(300, 401)" - ); - $old->safe_psql('testdb2', - "CREATE TABLE test4 AS SELECT generate_series(400, 502)"); + $tblspc = PostgreSQL::Test::Utils::tempdir_short(); } + $old->safe_psql('postgres', + "CREATE TABLESPACE test_tblspc LOCATION '$tblspc'"); + $old->safe_psql('postgres', + "CREATE DATABASE testdb2 TABLESPACE test_tblspc"); + $old->safe_psql('postgres', + "CREATE TABLE test3 TABLESPACE test_tblspc AS SELECT generate_series(300, 401)" + ); + $old->safe_psql('testdb2', + "CREATE TABLE test4 AS SELECT generate_series(400, 502)"); $old->stop; I would vote for having a total of two tablespaces when we can do so safely: one non-inplace and one inplace, strengthening the cross-checks for the prefixes assigned in the new paths when doing a swap of the tablespace paths, even when an old installation is tested. + /* + * For now, we do not expect non-in-place tablespaces to move during + * upgrade. If that changes, it will likely become necessary to run + * the above query on the new cluster, too. + */ Curiosity matter: has this ever been discussed? Spoiler: I don't recall so and why it would ever matter as tbspace OIDs should be fixed across upgrades these days (right?). @@ -53,19 +63,44 @@ get_tablespace_paths(void) [...] + if (is_absolute_path(PQgetvalue(res, tblnum, i_spclocation))) + { This use of is_absolute_path() should document that it is for in-place tablespaces? + if (strcmp(new_tablespace, new_cluster.pgdata) == 0) + new_tblspc_suffix = "/base"; This knowledge is encapsulated in GetDatabasePath() currently. Not new, just saying that it could be nice to reduce the footprint of this knowledge in pg_upgrade. Perhaps not something to worry about here, though. -- Michael -
Re: teach pg_upgrade to handle in-place tablespaces
Nathan Bossart <nathandbossart@gmail.com> — 2025-07-22T02:06:59Z
On Tue, Jul 22, 2025 at 10:37:05AM +0900, Michael Paquier wrote: > On Mon, Jul 21, 2025 at 07:57:32PM -0500, Nathan Bossart wrote: > + if (!defined($ENV{oldinstall})) > + { > + $new->append_conf('postgresql.conf', > + "allow_in_place_tablespaces = true"); > + $old->append_conf('postgresql.conf', > + "allow_in_place_tablespaces = true"); > + } > > This would not choke as long as the old cluster is at least at v10, > but well why not. I'm not sure what you mean. allow_in_place_tablespaces was added in v15, right? > I would vote for having a total of two tablespaces when we can do so > safely: one non-inplace and one inplace, strengthening the > cross-checks for the prefixes assigned in the new paths when doing a > swap of the tablespace paths, even when an old installation is tested. Seems reasonable, will add this. > + /* > + * For now, we do not expect non-in-place tablespaces to move during > + * upgrade. If that changes, it will likely become necessary to run > + * the above query on the new cluster, too. > + */ > > Curiosity matter: has this ever been discussed? Spoiler: I don't > recall so and why it would ever matter as tbspace OIDs should be fixed > across upgrades these days (right?). I'm not aware of any such discussion. I know there was a time before version-specific tablespace subdirectories were a thing, but that is long gone. And yeah, we've preserved tablespace OIDs since v15. > + if (is_absolute_path(PQgetvalue(res, tblnum, i_spclocation))) > + { > > This use of is_absolute_path() should document that it is for in-place > tablespaces? Yes. > + if (strcmp(new_tablespace, new_cluster.pgdata) == 0) > + new_tblspc_suffix = "/base"; > > This knowledge is encapsulated in GetDatabasePath() currently. Not > new, just saying that it could be nice to reduce the footprint of this > knowledge in pg_upgrade. Perhaps not something to worry about here, > though. I'll take a look and see what I can do. -- nathan -
Re: teach pg_upgrade to handle in-place tablespaces
Michael Paquier <michael@paquier.xyz> — 2025-07-22T02:17:42Z
On Mon, Jul 21, 2025 at 09:06:59PM -0500, Nathan Bossart wrote: > On Tue, Jul 22, 2025 at 10:37:05AM +0900, Michael Paquier wrote: >> This would not choke as long as the old cluster is at least at v10, >> but well why not. > > I'm not sure what you mean. allow_in_place_tablespaces was added in v15, > right? Yes initially, not so these days. Here are the backpatches across v10~14: <16e7a8fd8e97> 2022-07-27 [Alvaro Herrera] Allow "in place" tablespaces. <961cab0a5a90> 2022-07-27 [Alvaro Herrera] Allow "in place" tablespaces. <7bdbbb87340f> 2022-07-27 [Alvaro Herrera] Allow "in place" tablespaces. <258c896418bf> 2022-07-27 [Alvaro Herrera] Allow "in place" tablespaces. <ca347f5433ed> 2022-07-27 [Alvaro Herrera] Allow "in place" tablespaces. -- Michael
-
Re: teach pg_upgrade to handle in-place tablespaces
Nathan Bossart <nathandbossart@gmail.com> — 2025-07-24T18:44:50Z
On Tue, Jul 22, 2025 at 11:17:42AM +0900, Michael Paquier wrote: > On Mon, Jul 21, 2025 at 09:06:59PM -0500, Nathan Bossart wrote: >> On Tue, Jul 22, 2025 at 10:37:05AM +0900, Michael Paquier wrote: >>> This would not choke as long as the old cluster is at least at v10, >>> but well why not. >> >> I'm not sure what you mean. allow_in_place_tablespaces was added in v15, >> right? > > Yes initially, not so these days. Here are the backpatches across > v10~14: Ah, I see. Here's a new patch that should address all your feedback except for the part about "/base". I figure we can leave that for another patch. -- nathan
-
Re: teach pg_upgrade to handle in-place tablespaces
Nathan Bossart <nathandbossart@gmail.com> — 2025-07-30T15:52:03Z
Committed. -- nathan