Horton

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

Peter Smith <smithpb2250@gmail.com>

View thread View on postgr.es
From: Peter Smith <smithpb2250@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.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-08-18T07:02:36Z
Lists: pgsql-hackers 
On Fri, Aug 18, 2023 at 12:47 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Thu, Aug 17, 2023 at 2:10 PM Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > Here are some review comments for the first 2 patches.
> >
> >   /*
> > - * Fetch all libraries containing non-built-in C functions in this DB.
> > + * Fetch all libraries containing non-built-in C functions and
> > + * output plugins in this DB.
> >   */
> >   ress[dbnum] = executeQueryOrDie(conn,
> >   "SELECT DISTINCT probin "
> >   "FROM pg_catalog.pg_proc "
> >   "WHERE prolang = %u AND "
> >   "probin IS NOT NULL AND "
> > - "oid >= %u;",
> > + "oid >= %u "
> > + "UNION "
> > + "SELECT DISTINCT plugin "
> > + "FROM pg_catalog.pg_replication_slots "
> > + "WHERE wal_status <> 'lost' AND "
> > + "database = current_database() AND "
> > + "temporary IS FALSE;",
> >   ClanguageId,
> >   FirstNormalObjectId);
> >   totaltups += PQntuples(ress[dbnum]);
> >
> > ~
> >
> > Maybe it is OK, but it somehow seems like the new logic has been
> > jammed into the get_loadable_libraries() function for coding
> > convenience. For example, all the names (function names, variable
> > names, structure field names) are referring to "libraries", so the
> > plugin seems a bit out of place.
> >
>
> But the same name library (as plugin) should exist for the upgrade of
> slots. I feel doing it separately could either lead to a redundant
> code or a different way to achieve the same thing. Do you envision any
> problem which we are not seeing?
>

No problem. I'd misunderstood that the "plugin" referred to here is a
shared object file (aka library) name, so it does belong here after
all. I think the new comments could be made more clear about this
point though.

------
Kind Regards,
Peter Smith.
Fujitsu Australia

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.