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

> On Oct 4, 2021, at 10:58 AM, Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
>
> On Mon, Oct 4, 2021 at 8:10 AM Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)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 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.

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