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

On Wed, Oct 6, 2021 at 11:32 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> All of the decisions we're talking about here really have to do with
> determining the user's intent. I think that if the user says
> pg_amcheck --all, there's a good argument that they don't want us to
> check unlogged relations on a standby which will never be valid, or
> failed index builds which need not be valid. But even that is not
> necessarily true. If the user typed pg_amcheck -i
> some_index_that_failed_to_build, there is a pretty strong argument
> that they want us to check that index and maybe fail, not skip
> checking that index and report success without doing anything. I think
> it's reasonable to accept that unfortunate deviation from the user's
> intent in order to get the benefit of not failing for silly reasons
> when, as will normally be the case, somebody just tries to check the
> entire database, or some subset of tables and their corresponding
> indexes. In those cases the user pretty clearly only wants to check
> the valid things. So I agree, with some reservations, that excluding
> unlogged relations while in recovery and invalid indexes is probably
> the thing which is most likely to give the users what they want.
>
> But how can we possibly say that a user who specifies --heapallindexed
> doesn't really mean what they said?

I am pretty sure that I agree with you about all these details. We
need to tease them apart some more.

--heapallindexed doesn't complicate things for us at all. It changes
nothing about the locking considerations. It's just an additive thing,
some extra checks with the same basic underlying requirements. Maybe
you meant to say --parent-check, not --heapallindexed?

--parent-check does present us with the question of what to do in Hot
Standby mode, where it will surely fail (because it requires a
relation level ShareLock, etc). But I actually don't think it's
complicated: we must throw an error, because it's fundamentally not
something that will ever work (with any index). Whether the error
comes from pg_amcheck or amcheck proper doesn't seem important to me.

I think it's pretty clear that verify_heapam.c (from amcheck proper)
should just follow verify_nbtree.c when directly invoked against an
unlogged index in Hot Standby. That is, it should assume that the
relation has no storage, but still "verify" it conceptually. Just show
a NOTICE about it. Assume no storage to verify.

Finally, there is the question of what happens inside pg_amcheck (not
amcheck proper) deals with unlogged relations in Hot Standby mode.
There are two reasonable options: it can either "verify" the indexes
(actually just show those NOTICE messages), or skip them entirely. I
lean towards the former option, on the grounds that I don't think it
should be special-cased. But I don't feel very strongly about it.

--
Peter Geoghegan