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
