Re: missing LockBuffer(buffer, BUFFER_LOCK_SHARE) in trigger.c GetTupleForTrigger?

On Fri, Nov 30, 2012 at 6:21 PM, Andres Freund <andres(at)2ndquadrant(dot)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(at)2ndquadrant(dot)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