Re: Question about InvalidatePossiblyObsoleteSlot()

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Cc: Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, "suyu(dot)cmj" <mengjuan(dot)cmj(at)alibaba-inc(dot)com>, "bharath(dot)rupireddyforpostgres" <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Question about InvalidatePossiblyObsoleteSlot()
Date: 2025-10-29 07:02:08
Message-ID: aQG78KIzVNlpEkwJ@paquier.xyz
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Oct 28, 2025 at 11:53:26AM -0700, Masahiko Sawada wrote:
> On Tue, Oct 28, 2025 at 1:58 AM Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:

Apologies for not following closely what has been happening on this
thread. I have just not been paying much attention, and Sawada-san
has just poked at me about its existence today, also regarding the
fact that my fingerprints are all over the place (implying that I'm an
owner here, even in light of 105b2cb3361 that has introduced the trick
to bypass the creation of the annoying standby snapshot records that
distabilized the tests).

>> I see, you mean that the tests are stable now (thanks to 105b2cb3361) but
>> that we should still do something for "production" cases? (i.e not making use
>> of injection points).
>
> Yes. While it seems we might want to review the past discussion, if
> we've concluded such behavior is problematic behavior and could
> confuse users, we can do something like improving the
> invalidation/termination reports. Or we can do nothing if the current
> reporting is fine.

What do you mean exactly here? An improvement in the reports done
when the invalidations are kicked sounds separate to me, that may not
be something to touch when it comes to the back-branches.

Anyway, coming back to the patch, the argument of relying on the
slot's restart_lsn and the two xmins to restore the pre-818fefd8fd4
sounds good to me in light of 105b2cb3361.

Extra argument: Is the fact that RS_INVAL_WAL_REMOVED (and optionally
RS_INVAL_IDLE_TIMEOUT) is used only by the checkpointer something that
we may want to document in the shape of an assertion at the beginning
of InvalidateObsoleteReplicationSlots() with an extra condition based
on the backend type? Based on the fact that restart_lsn could be
updated across two loops of the invalidation logic done by the
checkpointer, the answer to my own question is "no" and we may want to
trigger this reason from other contexts. Just wondering if that's
worth doing, as it has been mentioned upthread.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2025-10-29 07:03:53 Re: remove pg_restrict workaround
Previous Message Amit Langote 2025-10-29 06:37:25 Re: Batching in executor