On Sat, Jan 19, 2019 at 4:02 AM Nishant, Fnu <nisha...@amazon.com> wrote:
>
> Hello,
>
> While locking heap pages (function RelationGetBufferForTuple() in file hio.c)
> we acquire locks in increasing block number order to avoid deadlock. In
> certain cases, we do have to drop and reacquire lock on heap pages (when we
> have to pin visibility map pages) to avoid doing IO while holding exclusive
> lock. The problem is we now acquire locks in bufferId order, 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 <rh...@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
