Horton

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

Dilip Kumar <dilipbalaut@gmail.com>

View thread View on postgr.es
From: Dilip Kumar <dilipbalaut@gmail.com>
To: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
Cc: Amit Kapila <amit.kapila16@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>, "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
Date: 2023-09-01T04:17:53Z
Lists: pgsql-hackers 
On Thu, Aug 31, 2023 at 7:56 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:

> Thanks for giving comments!

Thanks

> > 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?

LGTM

>
> > 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(*).

Yeah, that seems a better option.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

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.