Horton

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

Peter Geoghegan <pg@bowt.ie>

View thread
From: Peter Geoghegan <pg@bowt.ie>
To: Pavel Borisov <pashkin.elfe@gmail.com>
Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
Date: 2021-03-03T03:10:32Z
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

On Mon, Feb 8, 2021 at 2:46 AM Pavel Borisov <pashkin.elfe@gmail.com> wrote:
> Caveat: if the first entry on the next index page has a key equal to the key on a previous page AND all heap tid's corresponding to this entry are invisible, currently cross-page check can not detect unique constraint violation between previous index page entry and 2nd, 3d and next current index page entries. In this case, there would be a message that recommends doing VACUUM to remove the invisible entries from the index and repeat the check. (Generally, it is recommended to do vacuum before the check, but for the testing purpose I'd recommend turning it off to check the detection of visible-invisible-visible duplicates scenarios)

It's rather unlikely that equal values in a unique index will end up
on different leaf pages. It's really rare, in fact. This following
comment block from nbtinsert.c (which appears right before we call
_bt_check_unique()) explains why this is:

* It might be necessary to check a page to the right in _bt_check_unique,
* though that should be very rare.  In practice the first page the value ...

You're going to have to "couple" buffer locks in the style of
_bt_check_unique() (as well as keeping a buffer lock on "the first
leaf page a duplicate might be on" throughout) if you need the test to
work reliably. But why bother with that? The tool doesn't have to be
100% perfect at detecting corruption (nothing can be), and it's rather
unlikely that it will matter for this test. A simple test that doesn't
handle cross-page duplicates is still going to be very effective.

I don't think that it's acceptable for your new check to raise a
WARNING instead of an ERROR. I especially don't like that the new
unique checking functionality merely warns that the index *might* be
corrupt. False positives are always unacceptable within amcheck, and I
think that this is a false positive.

-- 
Peter Geoghegan