On Wed, Jul 27, 2016 at 3:24 AM, Robert Haas <robertmh...@gmail.com> wrote:
> On Sat, Jul 23, 2016 at 3:55 AM, Amit Kapila <amit.kapil...@gmail.com> wrote:
>> Attached patch fixes the problem for me.  Note, I have not tried to
>> reproduce the problem for heap_lock_updated_tuple_rec(), but I think
>> if you are convinced with above cases, then we should have a similar
>> check in it as well.
> I don't think this hunk is correct:
> +        /*
> +         * If we didn't pin the visibility map page and the page has become
> +         * all visible, we'll have to unlock and re-lock.  See 
> heap_lock_tuple
> +         * for details.
> +         */
> +        if (vmbuffer == InvalidBuffer && 
> PageIsAllVisible(BufferGetPage(buf)))
> +        {
> +            LockBuffer(buf, BUFFER_LOCK_UNLOCK);
> +            visibilitymap_pin(rel, block, &vmbuffer);
> +            LockBuffer(buf, BUFFER_LOCK_EXCLUSIVE);
> +            goto l4;
> +        }
> The code beginning at label l4 appears that the buffer is unlocked,
> but this code leaves the buffer unlocked.  Also, I don't see the point
> of doing this test so far down in the function.  Why not just recheck
> *immediately* after taking the buffer lock?

Right, in this case we can recheck immediately after taking buffer
lock, updated patch attached.  In the passing by, I have noticed that
heap_delete() doesn't do this unlocking, pinning of vm and locking at
appropriate place.  It just checks immediately after taking lock,
whereas in the down code, it do unlock and lock the buffer again.  I
think we should do it as in attached patch

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Attachment: pin_vm_lock_tuple-v2.patch
Description: Binary data

Attachment: pin_vm_heap_delete-v1.patch
Description: Binary data

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to