Re: BUG #17212: pg_amcheck fails on checking temporary relations

> On Oct 4, 2021, at 10:58 AM, Peter Geoghegan <pg@bowt.ie> wrote:
>
> 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.
>>
>> I believe this is a bug in amcheck's btree checking functions.  Peter, can you take a look?
>
> Why do you say that?

Because REINDEX CONCURRENTLY and the bt_index_parent_check() function seem to have lock upgrade hazards that are
unrelatedto 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
indexesthat 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,
printsfor the user's benefit any errors those checks raise, and finally returns 0 if they all pass and 2 otherwise.
Sincenot all relations could be checked, 2 is returned.  Returning 0 would be misleading, as it implies everything was
checkedand passed, and it can't honestly say that.  The return value 2 does not mean that anything failed.  It means
thatnot all checks passed.  When a 2 is returned, the user is expected to read the output and decide what, if anything,
theywant to do about it.  In this case, the user might decide to wait until the reindex finishes and check again, or
theymight 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
return2.  It can't return 0, and it is unhelpful to leave the user in the dark about the fact that not all indexes are
inthe 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
suchinformation, perhaps with not printing it as the default.  

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company