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

From: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
To: 'Dilip Kumar' <dilipbalaut(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Bruce Momjian <bruce(at)momjian(dot)us>, Julien Rouhaud <rjuju123(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>
Subject: RE: [PoC] pg_upgrade: allow to upgrade publisher node
Date: 2023-08-31 14:26:21
Message-ID: TYAPR01MB58665CC7BB7EC7C28DD85BBCF5E5A@TYAPR01MB5866.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Dear Dilip,

Thanks for giving comments!

> Some comments in 0002
>
> 1.
> + res = executeQueryOrDie(conn, "SELECT slot_name "
> + "FROM pg_catalog.pg_replication_slots "
> + "WHERE slot_type = 'logical' AND "
> + "temporary IS FALSE;");
>
> What is the reason we are ignoring temporary slots here? I think we
> better explain in the comments.

The temporary slots were expressly ignored while checking because such slots
cannot exist after the upgrade. Before doing pg_upgrade, both old and new cluster
must be turned off, and they start/stop several times during the upgrade.

How do you think?

> 2.
> + res = executeQueryOrDie(conn, "SELECT slot_name "
> + "FROM pg_catalog.pg_replication_slots "
> + "WHERE slot_type = 'logical' AND "
> + "temporary IS FALSE;");
> +
> + if (PQntuples(res))
> + pg_fatal("New cluster must not have logical replication slots but
> found \"%s\"",
> + PQgetvalue(res, 0, 0));
>
> It looks a bit odd to me that first it is fetching all the logical
> slots from the new cluster and then printing the name of one of the
> slots. If it is printing the name of the slots then shouldn't it be
> printing all the slots' names or it should just say that there
> existing slots on the new cluster without giving any names? And if we
> are planning for option 2 i.e. not printing the name then better to
> put LIMIT 1 at the end of the query.

I'm planning to change that the number of slots are reported by using count(*).

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dagfinn Ilmari Mannsåker 2023-08-31 15:12:14 Re: Replace some cstring_to_text to cstring_to_text_with_len
Previous Message Ranier Vilela 2023-08-31 14:11:44 Re: Replace some cstring_to_text to cstring_to_text_with_len