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.
Regards,
Vignesh