Re: possible deadlock: different lock ordering for heap pages

On Sat, Jan 19, 2019 at 4:02 AM Nishant, Fnu <nishantf@amazon.com> wrote:
>
> Hello,
>
> While locking heap pages (function RelationGetBufferForTuple() in file hio.c) we acquire locks in increasing block
numberorder to avoid deadlock. In certain cases, we do have to drop and reacquire lock on heap pages (when we have to
pinvisibility map pages) to avoid doing IO while holding exclusive lock. The problem is we now acquire locks in
bufferIdorder, which looks like a slip and the intention was to grab it in block number order. Since it is a trivial
change,I am attaching a patch to correct it. 
>

On a quick look, your analysis seems correct, but I am bit surprised
to see how this survived so long without being caught.  I think you
need to change below code as well if your analysis and fix is correct.
GetVisibilityMapPins()
{
..
Assert(buffer2 == InvalidBuffer || buffer1 <= buffer2);
..
}

AFAICS, this code has been added by below commit, so adding author in the loop.

commit e16954f3d27fa8e16c379ff6623ae18d6250a39c
Author: Robert Haas <rhaas@postgresql.org>
Date:   Mon Jun 27 13:55:55 2011 -0400

    Try again to make the visibility map crash safe.

    My previous attempt was quite a bit less than half-baked with respect to
    heap_update().

Robert, can you please once see if we are missing anything here
because to me the report and fix look genuine.


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