Horton

RE: [PoC] pg_upgrade: allow to upgrade publisher node

Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>

View thread View on postgr.es
From: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
To: 'Peter Smith' <smithpb2250@gmail.com>
Cc: Julien Rouhaud <rjuju123@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Amit Kapila <amit.kapila16@gmail.com>
Date: 2023-04-14T05:53:37Z
Lists: pgsql-hackers

Attachments

Dear Peter,

Thank you for checking. Then we can wait comments from others.
PSA modified version.

> 1.
> There were a couple of comments that I thought would appear less
> squished (aka more readable) if there was a blank line preceding the
> XXX.
> 
> 1a. This one is in getLogicalReplicationSlots
> 
> + /*
> + * Get replication slots.
> + *
> + * XXX: Which information must be extracted from old node? Currently three
> + * attributes are extracted because they are used by
> + * pg_create_logical_replication_slot().
> + * XXX: Do we have to support physical slots?
> + */

Added.

> 1b. This one is for the LogicalReplicationSlotInfo typedef
> 
> +/*
> + * The LogicalReplicationSlotInfo struct is used to represent replication
> + * slots.
> + * XXX: add more attrbutes if needed
> + */

Added.

> BTW -- I just noticed there is a typo in that comment. /attrbutes/attributes/

Good finding, replaced.

> src/bin/pg_dump/pg_dump_sort.c
> 
> 2. describeDumpableObject
> 
> + case DO_LOGICAL_REPLICATION_SLOT:
> + snprintf(buf, bufsize,
> + "REPLICATION SLOT (ID %d NAME %s)",
> + obj->dumpId, obj->name);
> + return;
> 
> Since everything else was changed to say logical replication slot,
> should this string be changed to "LOGICAL REPLICATION SLOT (ID %d NAME
> %s)"?

I missed to replace, changed.

> .../pg_upgrade/t/003_logical_replication.pl
> 
> 3.
> Should the name of this TAP test file really be 03_logical_replication_slots.pl?
>

Hmm, not sure. Currently I renamed once according to your advice, but personally
another feature which allows to upgrade subscriber[1] should be tested in the same
perl file. That's why I named as "003_logical_replication.pl"

[1]: https://www.postgresql.org/message-id/20230217075433.u5mjly4d5cr4hcfe%40jrouhaud


Best Regards,
Hayato Kuroda
FUJITSU LIMITED

Commits

  1. Fix issues in binary_upgrade_logical_slot_has_caught_up().

  2. Fix a random failure in 003_logical_slots.pl.

  3. Fix a test in 003_logical_slots.

  4. Fix uninitialized slot array access during the upgrade.

  5. Fix the test 003_logical_slots.

  6. Commit b195e6d482 forgot to update meson.build.

  7. Use shorter file names in the upgrade logical slots test.

  8. Migrate logical slots to the new node during an upgrade.

  9. Flush logical slots to disk during a shutdown checkpoint if required.

  10. Prevent possibility of panics during shutdown checkpoint.