Re: [PATCH] Full support for index LP_DEAD hint bits on standby

From: Michail Nikolaev <michail(dot)nikolaev(at)gmail(dot)com>
To: Peter Geoghegan <pg(at)bowt(dot)ie>
Cc: Greg Stark <stark(at)mit(dot)edu>, Andres Freund <andres(at)anarazel(dot)de>, Julien Rouhaud <rjuju123(at)gmail(dot)com>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, Antonin Houska <ah(at)cybertec(dot)at>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Full support for index LP_DEAD hint bits on standby
Date: 2022-03-29 11:55:04
Message-ID: CANtu0oi9qGSrfbpxSECY02wAJQVLYBV=LVYYNDfhJq6u5opb2Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello, Peter.

Thanks for your review!

> I doubt that the patch's use of pg_memory_barrier() in places like
> _bt_killitems() is correct. There is no way to know for sure if this
> novel new lockless algorithm is correct or not, since it isn't
> explained anywhere.

The memory barrier is used only to ensure memory ordering in case of
clearing LP_DEAD bits. Just to make sure the flag allowing the use
LP_DEAD is seen AFTER bits are cleared.
Yes, it should be described in more detail.
The flapping test is one added in the patch and not related to memory
ordering. I have already tried to make it stable once before, but it
depends on minRecoveryLSN propagation. I’ll think about how to make it
stable.

> If there is an LP_DEAD bit set on a posting list on the primary, and
> we need to do a posting list split against the posting tuple, we need
> to be careful -- we cannot allow our new TID to look like it's LP_DEAD
> immediately, before our transaction even commits/aborts. We cannot
> swap out our new TID with an old LP_DEAD TID, because we'll think that
> our new TID is LP_DEAD when we shouldn't.

Oh, good catch! I was thinking it is safe to have additional hint bits
on primary, but it seems like no. BTW I am wondering if it is possible
to achieve the same situation by pg_rewind and standby promotion…

> Overall, I think that this patch has serious design flaws, and that
> this issue is really just a symptom of a bigger problem.

Could you please advise me on something? The ways I see:
* give up :)
* try to fix this concept
* go back to concept with LP_DEAD horizon WAL and optional cancellation
* try to limit scope on “allow standby to use LP_DEAD set on primary
in some cases” (by marking something in btree page probably)
* look for some new way

Best regards,
Michail.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2022-03-29 12:08:26 Re: [PATCH] Enable SSL library detection via PQsslAttribute
Previous Message Michail Nikolaev 2022-03-29 11:52:44 Re: [PATCH] Full support for index LP_DEAD hint bits on standby