Thread: Re: bt_index_parent_check and concurrently build indexes

Re: bt_index_parent_check and concurrently build indexes

From
"Andrey M. Borodin"
Date:

> On 9 Dec 2024, at 23:51, Michail Nikolaev <michail.nikolaev@gmail.com> wrote:
>
> * Modify bt_index_parent_check to use an MVCC snapshot for the heap scan

Hi!

Interesting bug. It's amazing how long it stand, giving that it would be triggered by almost any check after updating a
table.

From my POV correct fix direction is to use approach similar to index building.
E.i. remove "if (!state->readonly)" check. Are there any known downsides of this?


Best regards, Andrey Borodin.


Re: bt_index_parent_check and concurrently build indexes

From
"Andrey M. Borodin"
Date:

> On 13 Dec 2024, at 04:59, Michail Nikolaev <michail.nikolaev@gmail.com> wrote:
> <v3-0001-amcheck-Fix-bt_index_parent_check-behavior-with-C.patch>

+# Copyright (c) 2021-2024, PostgreSQL Global Development Group

I think usually write only commit year. Something tells me you can safely write 2025 there.

+Test::More->builder->todo_start('filesystem bug')
+  if PostgreSQL::Test::Utils::has_wal_read_bug;

Can't wrap my head why do you need this?

+# it fails, because it is expect to find the deleted row in index

I think this comment describes behavior before the fix in present tense.

-        if (snapshot != SnapshotAny)
-            UnregisterSnapshot(snapshot);

Snapshot business seems incorrect to me here...


Best regards, Andrey Borodin.


Re: bt_index_parent_check and concurrently build indexes

From
Michael Paquier
Date:
On Mon, Jun 02, 2025 at 05:40:18PM -0700, Donghang Lin wrote:
> Your finding is right on point! We recently used bt_index_parent_check to
> verify concurrently built indexes in a concurrent workload,
> bt_index_parent_check often gave such false positive error.

Good thing is that this is tracked in the CF app:
https://commitfest.postgresql.org/patch/5438/

Peter, could you look at that?  amcheck and btree are both in your
area of expertise.  Getting this error because of routine CIC or
REINDEX CONCURRENTLY runs is annoying.
--
Michael

Attachment

Re: bt_index_parent_check and concurrently build indexes

From
Mihail Nikalayeu
Date:
Hello, Donghang!

> One suggestion to this change is that we might need to update the amcheck doc to reflect that
> "This consists of a “dummy” CREATE INDEX CONCURRENTLY operation" rather than "CREATE INDEX" operation.

+1, done. Also fixed some typos in the commit message.

Best regards,
Mikhail.

Attachment