Re: Buffer locking is special (hints, checksums, AIO writes)

Hi,

On 2026-01-24 16:11:47 -0500, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > I think this is more likely to be a spgist bug, not a bug in the patch. From
> > what I can tell, spgist tries to conditionally lock a buffer that it itself
> > already has locked exclusively - that's why the assertion is failing.
>
> I dunno. It looks to me like the previous LWLock-based implementation
> of ConditionalLockBuffer() had no such restriction as
>
> /*
> * We better not already hold a lock on the buffer.
> */
> Assert(entry->data.lockmode == BUFFER_LOCK_UNLOCK);
>
> Maybe I'm missing something, but it looks like the old code would
> return false if the buffer was already locked, whether that lock
> was held by our process or another one.

Yea. I added that assert when (I think) Melanie complained that the new code
wouldn't detect repeated acquisitions or release of the same content lock as
nicely as before. I guess I went a bit overboard and also added the assertion
to ConditionalLockBuffer().

I'll go and move it into the branch where we actually got the lock, that seems
worth continuing to do.

> > We could of course just accept this case and have the conditional lock
> > acquisition fail, but I think trying to conditionally lock a buffer that you
> > already lock is indicative of something having gone wrong.
>
> I don't really buy this argument. Yes, within a single function it'd
> be silly to lock a buffer and immediately try to lock it again, but
> when you consider cases like recursive modifications of index state,
> it's *far* from obvious that some lower recursion level might not try
> to lock a buffer that some outer level already locked.

Yea, there's probably something too that. But I'm not entirely convinced -
consider what happens with an index on a temporary table: A higher level think
it got a buffer with space, but then a lower level uses up that space...

> In the case at hand I think it is probably driven by two recursion levels
> trying to acquire free space out of the same buffer. SPGist is expecting
> the lower level to fail to get the lock and then go find some free space
> elsewhere. Yeah, we could probably re-code it to get that outcome in
> another way, but why?

Regardless of the assertion, it still feels like there may be something off
here. Why is the page marked as empty in the FSM, despite actually not being
empty?

Greetings,

Andres Freund