Re: Question about InvalidatePossiblyObsoleteSlot()

Bertrand Drouvot <bertranddrouvot.pg@gmail.com>

From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>, "suyu.cmj" <mengjuan.cmj@alibaba-inc.com>, michael <michael@paquier.xyz>, "bharath.rupireddyforpostgres" <bharath.rupireddyforpostgres@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2025-10-23T10:07:23Z
Lists: pgsql-hackers

Attachments

Hi,

On Wed, Oct 22, 2025 at 02:18:33PM +0530, Amit Kapila wrote:
> On Mon, Oct 20, 2025 at 11:41 AM Bertrand Drouvot
> <bertranddrouvot.pg@gmail.com> wrote:
> >
> > Yeah so 818fefd8fd4 is well suited for tests consistency but in some rare cases
> > it invalidates a slot while it would be safe not to do so.
> >
> > OTOH it looks to me that the initial pre-818fefd8fd4 intend was to invalidate
> > the slot as per this comment:
> >
> > "
> > /*
> >  * Re-acquire lock and start over; we expect to invalidate the
> >  * slot next time (unless another process acquires the slot in the
> >  * meantime).
> >  */
> > "
> >
> 
> The comment doesn't indicate the intent that we will invalidate the
> slot after re-acquiring the lock even when the new conditions don't
> warrant the slot to be invalidated. The comment could be improved
> though.

Thanks for looking at it and clarifying this point. In the attached I try
to improve the comment.

> > The fact that it could move forward far enough before we terminate the
> > process holding the slot is a race condition due to the fact that we released
> > the mutex.
> >
> > If the above looks right to you then 818fefd8fd4 is doing what was "initially"
> > expected, do you agree?
> >
> 
> Based on the discussion and points presented in this thread, I don't
> agree. I feel we should restore behavior prior to 818fefd8fd4

Done in the attached.

> and fix
> the test case which relies on different messages.

I think that 105b2cb3361 fixed it already or do you have something else in
mind?

[1]: https://www.postgresql.org/message-id/5d0e5bec-67f9-9164-36cb-c4ff5f95d1ed%40gmail.com

Regards,

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

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Fix regression with slot invalidation checks

  2. Fix race leading to incorrect conflict cause in InvalidatePossiblyObsoleteSlot()