RE: [PoC] pg_upgrade: allow to upgrade publisher node
Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>
From: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
To: 'Amit Kapila' <amit.kapila16@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>, Dilip Kumar <dilipbalaut@gmail.com>, Peter Smith <smithpb2250@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Bruce Momjian <bruce@momjian.us>, Julien Rouhaud <rjuju123@gmail.com>, vignesh C <vignesh21@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: 2023-10-17T12:15:04Z
Lists: pgsql-hackers
Attachments
- v51-0001-pg_upgrade-Allow-to-replicate-logical-replicatio.patch (application/octet-stream) patch v51-0001
- v51-0002-support-cross-version-upgrade.patch (application/octet-stream) patch v51-0002
Dear Amit,
Thanks for reviewing! PSA new version.
>
> Yeah, I think introducing additional complexity unless it is really
> required sounds a bit scary to me as well. BTW, please find attached
> some cosmetic changes.
Basically LGTM, but below part was conflicted with Bharath's comment [1].
```
@@ -1607,7 +1605,7 @@ check_old_cluster_for_valid_slots(bool live_check)
fclose(script);
pg_log(PG_REPORT, "fatal");
- pg_fatal("Your installation contains logical replication slots that cannot be upgraded.\n"
+ pg_fatal("Your installation contains invalid logical replication slots.\n"
```
How about " Your installation contains logical replication slots that can't be upgraded."?
> One minor additional comment:
> +# Initialize subscriber cluster
> +my $subscriber = PostgreSQL::Test::Cluster->new('subscriber');
> +$subscriber->init(allows_streaming => 'logical');
>
> Why do we need to set wal_level as logical for subscribers?
It is not mandatory. The line was copied from tests in src/test/subscription.
Removed the setting from my patch. I felt that it could be removed from other
patches. I will fork new thread and post the patch.
Also, I did some improvements based on the v50, basically for tests.
1. Test file was refactored. pg_uprade was executed many times in the test so the
test time was increasing. Below refactorings were done.
===
a. Checks for both transactional and non-transactional changes were done at the
same time.
b. Removed the dry-run test. It did not improve the coverage.
c. Removed the wal_level test. Other tests like subscriptions and test_decoding
do not contain test for GUCs, so I thought it could be acceptable. Removing
all the GUC test (for max_replication_slots) might be risky, so it was remained.
===
2. Supported the cross-version checks. If an environment variable "oldinstall"
is set, use the binary as old cluster. If the specified one is PG16-, the
test verifies that logical replication slots would not be migrated.
002_pg_upgrade.pl requires that $ENV(olddump) must be also defined, but it
is not needed for our test. I tried to support from PG9.2, which is the oldest
version for Xupgrade test [2]. You can see 0002 patch for it.
IIUC pg_create_logical_replication_slot() can be available since PG9.4, so tests
will be skipped if older executables are specified, like:
```
$ oldinstall=/home/hayato/older/pg92/ make check PROVE_TESTS='t/003_upgrade_logical_replication_slots.pl'
...
# +++ tap check in src/bin/pg_upgrade +++
t/003_upgrade_logical_replication_slots.pl .. skipped: Logical replication slots can be available since PG9.4
Files=1, Tests=0, 0 wallclock secs ( 0.03 usr 0.00 sys + 0.08 cusr 0.02 csys = 0.13 CPU)
Result: NOTESTS
```
[1]: https://www.postgresql.org/message-id/CALj2ACXp%2BLXioY_%3D9mboEbLD--4c4nnpJCZ%2Bj4fckBdSOQhENA%40mail.gmail.com
[2]: https://github.com/PGBuildFarm/client-code/releases#:~:text=support%20for%20testing%20cross%20version%20upgrade%20extended%20back%20to%209.2
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Commits
-
Fix issues in binary_upgrade_logical_slot_has_caught_up().
- 0bf62460bb9e 17.0 landed
-
Fix a random failure in 003_logical_slots.pl.
- 63c5df126abb 17.0 landed
-
Fix a test in 003_logical_slots.
- 3e36e48d8efe 17.0 landed
-
Fix uninitialized slot array access during the upgrade.
- a7db71ed2787 17.0 landed
-
Fix the test 003_logical_slots.
- 8af917be6bad 17.0 landed
-
Commit b195e6d482 forgot to update meson.build.
- 8949b978ff49 17.0 landed
-
Use shorter file names in the upgrade logical slots test.
- b195e6d482b8 17.0 landed
-
Migrate logical slots to the new node during an upgrade.
- 29d0a77fa660 17.0 landed
-
Flush logical slots to disk during a shutdown checkpoint if required.
- e0b2eed047df 17.0 landed
-
Prevent possibility of panics during shutdown checkpoint.
- c6c333436491 10.0 cited