Re: Concurrency bug in amcheck

On Mon, Apr 27, 2020 at 11:51 AM Alexander Korotkov
<a(dot)korotkov(at)postgrespro(dot)ru> wrote:
> On Wed, Apr 22, 2020 at 7:47 PM Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
> > On Tue, Apr 21, 2020 at 2:54 AM Alexander Korotkov
> > <a(dot)korotkov(at)postgrespro(dot)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