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

From: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
To: "Wei Wang (Fujitsu)" <wangw(dot)fnst(at)fujitsu(dot)com>, 'Peter Smith' <smithpb2250(at)gmail(dot)com>
Cc: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Julien Rouhaud <rjuju123(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Subject: RE: [PoC] pg_upgrade: allow to upgrade publisher node
Date: 2023-05-11 08:55:08
Message-ID: TYAPR01MB5866A3B91F56056A803B94DAF5749@TYAPR01MB5866.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Dear Wang,

Thank you for reviewing! PSA new version.

> 1. In the function getLogicalReplicationSlots
> ```
> + /*
> + * Note: Currently we do not have any options to include/exclude
> slots
> + * in dumping, so all the slots must be selected.
> + */
> + slotinfo[i].dobj.dump = DUMP_COMPONENT_ALL;
> ```
> I think currently we are only dumping the definition of logical replication
> slots. It seems better to set it as DUMP_COMPONENT_DEFINITION here.

Right. Actually it was harmless because another flags like DUMP_COMPONENT_DEFINITION
are not checked in dumpLogicalReplicationSlot(), but changed.

> 2. In the function dumpLogicalReplicationSlot
> ```
> + ArchiveEntry(fout, slotinfo->dobj.catId, slotinfo->dobj.dumpId,
> + ARCHIVE_OPTS(.tag = slotname,
> +
> .description = "REPLICATION SLOT",
> + .section =
> SECTION_POST_DATA,
> +
> .createStmt = query->data));
> ```
> I think if we do not set the member dropStmt in macro ARCHIVE_OPTS here, when
> we
> specifying the option "--logical-replication-slots-only" and option "-c/--clean"
> together, the "-c/--clean" will not work.
>
> I think that we could use the function pg_drop_replication_slot to set this
> member. Then, in the main function in the pg_dump.c file, we should add a check
> to prevent specifying option "--logical-replication-slots-only" and
> option "--if-exists" together.
> Or, we could simply add a check to prevent specifying option
> "--logical-replication-slots-only" and option "-c/--clean" together.
> What do you think?

I chose not to allow to combine with -c. Assuming that this option is used only
by the pg_upgrade, it is ensured that new node does not have any logical replication
slots. So the remove function is not needed.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

Attachment Content-Type Size
v12-0001-pg_upgrade-Add-include-logical-replication-slots.patch application/octet-stream 32.1 KB
v12-0002-Always-persist-to-disk-logical-slots-during-a-sh.patch application/octet-stream 4.8 KB
v12-0003-pg_upgrade-Add-check-function-for-include-logica.patch application/octet-stream 5.7 KB
v12-0004-Change-the-method-used-to-check-logical-replicat.patch application/octet-stream 7.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Hayato Kuroda (Fujitsu) 2023-05-11 08:56:04 RE: [PoC] pg_upgrade: allow to upgrade publisher node
Previous Message Alvaro Herrera 2023-05-11 08:36:30 Re: [PATCH] Allow Postgres to pick an unused port to listen