Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.7\))
Subject: Re: BUG #17212: pg_amcheck fails on checking temporary relations
From: Mark Dilger <mark.dilger@enterprisedb.com>
In-Reply-To: 
 <CAH2-Wz=v4py=c79J7wpdVmQMkd8rO7gj5Ur5Dp4E0y7aQUz6cg@mail.gmail.com>
Date: Mon, 4 Oct 2021 15:36:06 -0700
Cc: Alexander Lakhin <exclusion@gmail.com>,
 PostgreSQL-development <pgsql-hackers@postgresql.org>,
 Robert Haas <robertmhaas@gmail.com>
Content-Transfer-Encoding: quoted-printable
Message-Id: <386F66EE-DEFD-42EA-9DB2-102BF3414669@enterprisedb.com>
References: <17212-34dd4a1d6bba98bf@postgresql.org>
 <CAH2-Wz=bw-Uk2yoOq7yevC9hyMGgZ2Ud34nd9YJJPXJ3pkVcrw@mail.gmail.com>
 <E85855BD-C063-4B24-9EAA-02DECC8F42B9@enterprisedb.com>
 <E283B45F-5745-49C9-BBC7-99876E2FABD1@enterprisedb.com>
 <f42de8ec-f219-0922-93b3-37a72bba1feb@gmail.com>
 <28B36E41-1E8F-4DC1-A924-711621BA4972@enterprisedb.com>
 <CAH2-Wz=v4py=c79J7wpdVmQMkd8rO7gj5Ur5Dp4E0y7aQUz6cg@mail.gmail.com>
To: Peter Geoghegan <pg@bowt.ie>
Archived-At: 
 <https://www.postgresql.org/message-id/386F66EE-DEFD-42EA-9DB2-102BF3414669%40enterprisedb.com>
Precedence: bulk


> On Oct 4, 2021, at 10:58 AM, Peter Geoghegan <pg@bowt.ie> wrote:
>=20
> On Mon, Oct 4, 2021 at 8:10 AM Mark Dilger =
<mark.dilger@enterprisedb.com> wrote:
>>> There is another issue, that maybe should be discussed separately =
(or
>>> this thread could be renamed to "... on checking specific =
relations"),
>>> but the solution could be similar to that.
>>> pg_amcheck also fails on checking invalid indexes, that could be =
created
>>> legitimately by the CREATE INDEX CONCURRENTLY command.
>>=20
>> I believe this is a bug in amcheck's btree checking functions.  =
Peter, can you take a look?
>=20
> Why do you say that?

Because REINDEX CONCURRENTLY and the bt_index_parent_check() function =
seem to have lock upgrade hazards that are unrelated to pg_amcheck.

> This hasn't been a
> problem before now, probably because the sample verification query in
> the docs (under bt_index_check()) accounts for this directly.

It doesn't say anything about deadlocks, but yes, it mentions errors =
will be raised unless the caller filters out indexes that are invalid or =
not ready.


On to pg_amcheck's behavior....

I see no evidence in the OP's complaint that pg_amcheck is misbehaving.  =
It launches a worker to check each relation, prints for the user's =
benefit any errors those checks raise, and finally returns 0 if they all =
pass and 2 otherwise.  Since not all relations could be checked, 2 is =
returned.  Returning 0 would be misleading, as it implies everything was =
checked and passed, and it can't honestly say that.  The return value 2 =
does not mean that anything failed.  It means that not all checks =
passed.  When a 2 is returned, the user is expected to read the output =
and decide what, if anything, they want to do about it.  In this case, =
the user might decide to wait until the reindex finishes and check =
again, or they might decide they don't care.

It is true that pg_amcheck is calling bt_index_parent_check() on an =
invalid index, but so what?  If it chose not to do so, it would still =
need to print a message about the index being unavailable for checking, =
and it would still have to return 2.  It can't return 0, and it is =
unhelpful to leave the user in the dark about the fact that not all =
indexes are in the right state for checking.  So it would still print =
the same error message and still return 2.

I think this bug report is really a feature request.  The OP appears to =
want an option to toggle on/off the printing of such information, =
perhaps with not printing it as the default.=20

=E2=80=94
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company