Re: missing LockBuffer(buffer, BUFFER_LOCK_SHARE) in trigger.c GetTupleForTrigger? - Mailing list pgsql-hackers

From Pavan Deolasee
Subject Re: missing LockBuffer(buffer, BUFFER_LOCK_SHARE) in trigger.c GetTupleForTrigger?
Date
Msg-id CABOikdM-9MrJs207Z8dnibss_VujMAGLYoteU0rtS-gPKmtV_g@mail.gmail.com
Whole thread Raw
In response to Re: missing LockBuffer(buffer, BUFFER_LOCK_SHARE) in trigger.c GetTupleForTrigger?  (Andres Freund <andres@2ndquadrant.com>)
Responses Re: missing LockBuffer(buffer, BUFFER_LOCK_SHARE) in trigger.c GetTupleForTrigger?
List pgsql-hackers


On Fri, Nov 30, 2012 at 5:28 PM, Andres Freund <andres@2ndquadrant.com> wrote:

> >
> That seems to be safe to me. Anything thats been read above can't really
> change. The tuple is already locked, so a concurrent update/delete has to
> wait on us. We have a pin on the buffer, so VACUUM or HOT-prune can't
> happen either. I can't see any other operation that can really change those
> fields.

We only get the pin right there, I don't see any preexisting pin. Which
means we might have just opened a page thats in the process of being
pruned/vacuumed by another backend.

Hmm. Yeah, you're right. That is a possible risky scenario. Even though cleanup lock waits for all pins to go away, it will work only if every reader takes at least a SHARE lock unless it was continuously holding a pin on a buffer (in which case its OK to drop lock and read a tuple body without reacquiring it again). Otherwise, as you rightly pointed out, we could actually be reading a page which being actively cleaned up and tuples are being moved around.
 
I think a concurrent heap_(insert|update)/PageAddItem might actually be
already dangerous because it might move line pointers around


I don't we move the line pointers around ever because the indexes will be pointing to them. But the vacuum/prune is dangerous enough to require a SHARE lock here in any case.
 
Thanks,

pgsql-hackers by date:

Previous
From: Alvaro Herrera
Date:
Subject: Re: Re: missing LockBuffer(buffer, BUFFER_LOCK_SHARE) in trigger.c GetTupleForTrigger?
Next
From: Dimitri Fontaine
Date:
Subject: Re: Review: Extra Daemons / bgworker