Horton

Re: [PATCH] Improve amcheck to also check UNIQUE constraint in btree index.

Pavel Borisov <pashkin.elfe@gmail.com>

View thread
From: Pavel Borisov <pashkin.elfe@gmail.com>
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: Peter Eisentraut <peter@eisentraut.org>, Aleksander Alekseev <aleksander@timescale.com>, Postgres hackers <pgsql-hackers@lists.postgresql.org>, Maxim Orlov <orlovmg@gmail.com>, Andres Freund <andres@anarazel.de>, Greg Stark <stark@mit.edu>, Julien Rouhaud <rjuju123@gmail.com>, Mark Dilger <mark.dilger@enterprisedb.com>, David Steele <david@pgmasters.net>, Peter Geoghegan <pg@bowt.ie>, Maxim Orlov <m.orlov@postgrespro.ru>, lubennikovaav@gmail.com
Date: 2024-04-25T12:59:54Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. amcheck: Optimize speed of checking for unique constraint violation

  2. amcheck: Report an error when the next page to a leaf is not a leaf

  3. amcheck: Don't load the right sibling page into BtreeCheckState

  4. amcheck: Refactoring the storage of the last visible entry

  5. Teach contrib/amcheck to check the unique constraint violation

  6. Add macros in hash and btree AMs to get the special area of their pages

Attachments

Hi, hackers!

On Wed, 24 Apr 2024 at 13:58, Alexander Korotkov <aekorotkov@gmail.com>
wrote:

> On Wed, Apr 17, 2024 at 9:38 AM Peter Eisentraut <peter@eisentraut.org>
> wrote:
> > On 24.10.23 22:13, Alexander Korotkov wrote:
> > > On Wed, Sep 28, 2022 at 11:44 AM Aleksander Alekseev
> > > <aleksander@timescale.com> wrote:
> > >>> I think, this patch was marked as "Waiting on Author", probably, by
> mistake. Since recent changes were done without any significant code
> changes and CF bot how happy again.
> > >>>
> > >>> I'm going to move it to RfC, could I? If not, please tell why.
> > >>
> > >> I restored the "Ready for Committer" state. I don't think it's a good
> > >> practice to change the state every time the patch has a slight
> > >> conflict or something. This is not helpful at all. Such things happen
> > >> quite regularly and typically are fixed in a couple of days.
> > >
> > > This patch seems useful to me.  I went through the thread, it seems
> > > that all the critics are addressed.
> > >
> > > I've rebased this patch.   Also, I've run perltidy for tests, split
> > > long errmsg() into errmsg(), errdetail() and errhint(), and do other
> > > minor enchantments.
> > >
> > > I think this patch is ready to go.  I'm going to push it if there are
> > > no objections.
> >
> > I just found the new pg_amcheck option --checkunique in PG17-to-be.
> > Could we rename this to --check-unique?  Seems friendlier.  Maybe also
> > rename the bt_index_check function argument to check_unique.
>
> +1 from me
> Let's do so if nobody objects.
>

Thank you very much for your input in this thread!

See the patches based on the proposals in the attachment:

0001: Optimize speed by avoiding heap visibility checking for different
non-deduplicated index tuples as proposed by Noah Misch

Speed measurements on my laptop using the exact method recommended by Noah
upthread:
Current master branch: checkunique off: 144s, checkunique on: 419s
With patch 0001: checkunique off: 141s, checkunique on: 171s

0002: Use structure to store and transfer info about last visible heap
entry (code refactoring) as proposed by Alexander Korotkov

0003: Don't load rightpage into BtreeCheckState (code refactoring) as
proposed by Peter Geoghegan

Loading of right page for cross-page unique constraint check in the same
way as in bt_right_page_check_scankey()

0004: Report error when next page to a leaf is not a leaf as proposed by
Peter Geoghegan

I think it's a very improbable condition and this check might be not
necessary, but it's right and safe to break check and report error.

0005: Rename checkunique parameter to more user friendly as proposed by
Peter Eisentraut and Alexander Korotkov

Again many thanks for the useful proposals!

Regards,
Pavel Borisov,
Supabase