Re: Concurrency bug in amcheck

On Mon, Apr 27, 2020 at 11:51 AM Alexander Korotkov
<a.korotkov@postgrespro.ru> wrote:
> On Wed, Apr 22, 2020 at 7:47 PM Peter Geoghegan <pg@bowt.ie> wrote:
> > On Tue, Apr 21, 2020 at 2:54 AM Alexander Korotkov
> > <a.korotkov@postgrespro.ru> wrote:
> > > Proposed fix is attached.  Spotted by Konstantin Knizhnik,
> > > reproduction case and fix from me.
> >
> > I wonder if we should fix btree_xlog_unlink_page() instead of amcheck.
> > We already know that its failure to be totally consistent with the
> > primary causes problems for backwards scans -- this problem can be
> > fixed at the same time:
> >
> > https://postgr.es/m/CANtu0ohkR-evAWbpzJu54V8eCOtqjJyYp3PQ_SGoBTRGXWhWRw@mail.gmail.com
> >
> > We'd probably still use your patch for the backbranches if we went this way.
> >
> > What do you think?
>
> I've skip through the thread.  It seems to be quite independent issue
> from this one.  This issue is related to the fact that we leave some
> items on deleted pages on primary, and on the same time we have no
> items on deleted pages on standby.  This inconsistency cause amcheck
> pass normally on primary, but fail on standby.  BackwardScan on
> standby issue seems to be related solely on locking protocol and
> btpo_prev, btpo_next pointers.  It wasn't mention on that thread that
> we might need hikeys on deleted pages.
>
> Assuming it doesn't seem we actually need any items on deleted pages,
> I can propose to delete them on primary as well.  That would make
> contents of primary and standby more consistent.  What do you think?

So, my proposal is following.  Backpatch my fix upthread to 11.  In
master additionally make _bt_unlink_halfdead_page() remove page items
on primary as well.  Do you have any objections?

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company