Horton

Re: Using Valgrind to detect faulty buffer accesses (no pin or buffer content lock held)

Anastasia Lubennikova <a.lubennikova@postgrespro.ru>

View thread View on postgr.es
From: Anastasia Lubennikova <a.lubennikova@postgrespro.ru>
To: Peter Geoghegan <pg@bowt.ie>, Daniel Gustafsson <daniel@yesql.se>
Cc: Andres Freund <andres@anarazel.de>, Alvaro Herrera <alvherre@2ndquadrant.com>, Masahiko Sawada <masahiko.sawada@2ndquadrant.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2020-07-16T17:24:51Z
Lists: pgsql-hackers 
On 02.07.2020 20:11, Peter Geoghegan wrote:
> On Thu, Jul 2, 2020 at 7:48 AM Daniel Gustafsson <daniel@yesql.se> wrote:
>> This patch fails to apply to HEAD due to conflicts in nbtpage.c, can you please
>> submit a rebased version?
> I attach the rebased patch series.
>
> Thanks

It's impressive that this check helped to find several bugs.

I only noticed small inconsistency in the new comment for 
_bt_conditionallockbuf().

It says "Note: Caller is responsible for calling _bt_checkpage() on 
success.", while in _bt_getbuf() the call is not followed by 
_bt_checkpage().
Moreover, _bt_page_recyclable() contradicts _bt_checkpage() checks.

Other than that, patches look good to me, so move them to "Ready For 
Committer".

Are you planning to add same checks for other access methods?

-- 
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Commits

  1. Add nbtree Valgrind buffer lock checks.

  2. Add Valgrind buffer access instrumentation.

  3. Fix bug in nbtree VACUUM "skip full scan" feature.

  4. Fix another minor page deletion buffer lock issue.

  5. Fix minor nbtree page deletion buffer lock issue.

  6. Skip full index scan during cleanup of B-tree indexes when possible