Re: Missing import in 035_standby_logical_decoding.pl

Hi,

On Sat, Aug 02, 2025 at 08:09:14AM +0900, Michael Paquier wrote:
> On Fri, Aug 01, 2025 at 03:22:06PM -0700, Masahiko Sawada wrote:
> > On Fri, Aug 1, 2025 at 1:58 PM Melanie Plageman
> > <melanieplageman@gmail.com> wrote:
> >> Another issue I see is that there are two incorrect tests for a log message
> >>
> >> # message should not be issued
> >> ok( !$node_standby->log_contains(
> >>         "invalidating obsolete slot \"no_conflict_inactiveslot\"", $logstart),
> >>     'inactiveslot slot invalidation is not logged with vacuum on conflict_test'
> >> );
> >>
> >> ok( !$node_standby->log_contains(
> >>         "invalidating obsolete slot \"no_conflict_activeslot\"", $logstart),
> >>     'activeslot slot invalidation is not logged with vacuum on conflict_test'
> >> );
> >>
> >> This will never fail because the message string is actually
> >> "invalidating obsolete replication slot"
> >>
> >> It is correctly specified elsewhere in the same test.
> >> so, that should be fixed too.
> > 
> > +1
> 
> Not the author of these two ones, but am I the only one who does not
> see the point of these two tests?  These check that something does not
> happen,

Yeah, we want to check that no conflicts occur for non catalog table.

> but log_contains() is useful to make sure that an event
> *actually happens*.

Not sure why it could not be useful to check that an event does not happen (
019_replslot_limit.pl also uses it that way).

> Wouldn't it be simpler to remove these checks

I'm afraid that could lead to false positives or to the test taking longer as
waiting for a timeout to occur.

> or
> switch them to use/add a log message that's triggered with a state
> that we'd expect to happen in reality?

If no conflicts occur, we do not emit messages in the log, then having nothing
to check. I think that's fine that way.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com