Re: amcheck verification for GiST

From: Peter Geoghegan <pg(at)bowt(dot)ie>
To: Alvaro Herrera from 2ndQuadrant <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: amcheck verification for GiST
Date: 2019-09-07 01:26:04
Message-ID: CAH2-WznzhHKREeXs1RMFYes+Ao7aam5dPSmvS2Hto6WUcjEk4A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Sep 6, 2019 at 3:22 PM Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
> I'll take care of it, then.

Attached is v10, which has some comment and style fix-ups, including
the stuff Alvaro mentioned. It also adds line pointer sanitization to
match what I added to verify_nbtree.c in commit a9ce839a (we use a
custom PageGetItemIdCareful() for GiST instead of a simple
PageGetItemId()). I also added a new file/TU for the routines that are
now common to both nbtree and GiST verification, which I named
amcheck.c. (I'm not sure about that, but I don't like verify_nbtree.c
having generic/common functions.)

I have only had a few hours to polish this, which doesn't seem like
enough, though was enough to fix the noticeable stuff.

My main concern now is the heavyweight lock strength needed by the new
function. I don't feel particularly qualified to sign off on the
concurrency aspects of the patch. Heikki's v6 used a ShareLock, like
bt_index_parent_check(), but you went back to an AccessShareLock,
Andrey. Why is this safe? I see that you do gist_refind_parent() in
your v9 a little differently to Heikki in his v6, which you seemed to
suggest made this safe in your e-mail on March 28, but I don't
understand that at all.

--
Peter Geoghegan

Attachment Content-Type Size
v10-0001-Revisions-of-GiST-amcheck-from-Peter.patch application/octet-stream 27.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2019-09-07 02:52:43 Re: pg_regress restart cluster?
Previous Message Joe Conway 2019-09-07 01:09:36 Re: add a MAC check for TRUNCATE