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

Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>

From: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
To: vignesh C <vignesh21@gmail.com>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
Cc: Peter Smith <smithpb2250@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Andres Freund <andres@anarazel.de>, Dilip Kumar <dilipbalaut@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Bruce Momjian <bruce@momjian.us>, Julien Rouhaud <rjuju123@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: 2023-10-20T15:21:23Z
Lists: pgsql-hackers
On Friday, October 20, 2023 11:24 AM vignesh C <vignesh21@gmail.com> wrote:
> 
> On Thu, 19 Oct 2023 at 16:16, Hayato Kuroda (Fujitsu)
> <kuroda.hayato@fujitsu.com> wrote:
> >
> > Dear Vignesh,
> >
> > Thanks for revieing! New patch can be available in [1].
> >
> > > Few comments:
> > > 1) Even if we comment 3rd point "Emit a non-transactional message",
> > > test_slot2 still appears in the
> > > invalid_logical_replication_slots.txt
> > > file. There is something wrong here.
> > > +       # 2. Advance the slot test_slot2 up to the current WAL location,
> but
> > > +       #        test_slot1 still has unconsumed WAL records.
> > > +       $old_publisher->safe_psql('postgres',
> > > +               "SELECT pg_replication_slot_advance('test_slot2',
> > > + NULL);");
> > > +
> > > +       # 3. Emit a non-transactional message. test_slot2 detects
> > > + the message
> > > so
> > > +       #        that this slot will be also reported by upcoming
> pg_upgrade.
> > > +       $old_publisher->safe_psql('postgres',
> > > +               "SELECT count(*) FROM
> > > + pg_logical_emit_message('false',
> > > 'prefix', 'This is a non-transactional message');"
> > > +       );
> >
> > The comment was updated based on others. How do you think?
> 
> I mean if we comment or remove this statement like in the attached patch, the
> test is still passing with 'The slot "test_slot2" has not consumed the WAL yet', in
> this case should the test_slot2 be still invalid as we have called
> pg_replication_slot_advance for test_slot2.

It's because we pass NULL to pg_replication_slot_advance(). We should pass 
pg_current_wal_lsn() instead. I have fixed it in V55 version.

Best Regards,
Hou zj

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.