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 CABOikdOEQKPHrbEHZ5T3qE95yAM_j04Beye5t_VRswjhb=GX5w@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?  (Andres Freund <andres@2ndquadrant.com>)
List pgsql-hackers


On Fri, Nov 30, 2012 at 6:21 PM, Andres Freund <andres@2ndquadrant.com> wrote:
On 2012-11-30 18:09:49 +0530, Pavan Deolasee wrote:
> 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.

Well, live (or recently dead) tuples can't be just moved arround unless
I miss something. That would cause problems with with HeapTuples
directly pointing into the page as youve noticed.


I think we confusing with the terminology. The tuple headers and bodies (live or dead) can be moved around freely as long as you have a cleanup lock on the page. Thats how HOT-prune frees up fragmented space.
 
> > 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.

The indexes point to a tid, not to a line pointer. So reshuffling the
linepointer array - while keeping the tids valid - is entirely
fine. Right?

I think its again terminology confusion :-) I thought TID and Line Pointers are the same and used interchangeably. Or at least I've done so for long. But I think you are reading line pointer as the thing stored in the ItemId structure and not the ItemId itself.
 
Also, PageAddItem() doesn't move things around normally. It does that only during recovery, at least for heao pages. Elsewhere it will either reuse an UNUSED pointer or add at the end. Isn't that how it is ?

Thanks,
Pavan

--
Pavan Deolasee
http://www.linkedin.com/in/pavandeolasee

pgsql-hackers by date:

Previous
From: Dimitri Fontaine
Date:
Subject: Re: Review: Extra Daemons / bgworker
Next
From: Peter Eisentraut
Date:
Subject: Re: [PATCH] Patch to fix a crash of psql