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

> On Oct 6, 2021, at 3:20 PM, Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
>
>> I think the disagreements are about something else.
>
> Informally speaking, you could say that pg_amcheck and amcheck verify
> relations. More formally speaking, both amcheck (whether called by
> pg_amcheck or some other thing) can only prove the presence of
> corruption. They cannot prove its absence. (The amcheck docs have
> always said almost these exact words.)

I totally agree that the amcheck functions cannot prove the absence of corruption.

I prefer not to even use language about proving the presence of corruption when discussing pg_amcheck. I have let that slide upthread as a convenient short-hand, but I think it doesn't help. For pg_amcheck to take any view whatsoever on whether a btree index is corrupt, it would have to introspect the error message that it gets back from bt_index_check(). It doesn't do that, nor do I think that it should. It just prints the contents of the error for the user and records that fact and eventually exits with a non-zero exit code. The error might have been something about the command exiting due to the crash of another backend, or to do with a deadlock against some other process, or whatever, and pg_amcheck has no opinion about whether any of that is to do with corruption or not.

> This seems to come up a lot because at various points you seem to be
> concerned about introducing specific imperfections. But it's not like
> your starting point was ever perfection, or ever could be.

From the point of view of detecting corruptions, I agree that it never could be. But I'm not talking about that. I'm talking about whether pg_amcheck issues all the commands that it is supposed to issue. If I work for Daddy Warbucks and he gives me 30 classic cars to take to 10 different mechanics, I can do that job perfectly even if the mechanics do less than perfect work. If I leave three cars in the driveway, that's on me. Likewise, it's not on pg_amcheck if the checking functions can't do perfect work, but it is on pg_amcheck if it doesn't issue all the expected commands. But later on in this email, it appears we don't have any remaining disagreements about that. Read on....

> I can
> always describe a scenario in which amcheck misses real corruption --
> a scenario which may be very contrived. So the mere fact that some new
> theoretical possibility of corruption is introduced by some action
> does not in itself mean much. We're dealing with that constantly, and
> always will be.

I wish we could stop discussing this. I really don't think this ticket has anything to do with how well or how poorly or how completely the amcheck functions work.

> Let's suppose I was to "directly fix amcheck + !indisvalid indexes". I
> don't even know what that means -- I honestly don't have a clue.
> You're focussing on one small piece of code in verify_nbtree.c, that
> seems to punt responsibility, but the fact is that there are deeply
> baked-in reasons why it does so. That's a reflection of how many
> things about the system work, in general. Attributing blame to any one
> small snippet of code (code in verify_nbtree.c, or wherever) just
> isn't helpful.

I think we have agreed that pg_amcheck can filter out invalid indexes. I don't have a problem with that. I admit that I did have a problem with that upthread, but its been a while since I conceded that point so I'd rather not have to argue it again.

>> In truth, all the pg_amcheck frontend client can take a view on is whether it was able to issue all the commands to the backend that it was asked to issue, and whether any of those commands responded with an error.
>
> AFAICT that pg_amcheck has to do is follow the amcheck user docs, by
> generalizing from the example SQL query for the B-Tree stuff. And, it
> should separately filter non-temp relations for the heap stuff, for
> the same reasons (exactly the same situation there).

I think we have agreed on that one, too, without me having ever argued it. I posted a patch to filter out the temporary tables already.

>> pg_amcheck -d db1 -d db2 -d db3 --table=mytable
>>
>> In this case, mytable is a regular table on db1, a temporary table on db2, and an unlogged table on db3, and db3 is in recovery.
>
> I don't think that pg_amcheck needs to care about being in recovery,
> at all. I agreed with you about using pg_is_in_recovery() from at one
> point. That was a mistake on my part.

Ok, excellent, that was probably the only thing that had me really hung up. I thought you were still asking for pg_amcheck to filter out the --parent-check option when in recovery, but if you're not asking for that, then I might have enough to go on now.

>> I thought that we were headed toward a decision where (despite my discomfort) pg_amcheck would downgrade options as necessary, but now it sounds like that's not so. So what should it do
>
> Downgrade is how you refer to it. I just think of it as making sure
> that pg_amcheck only asks amcheck to verify relations that are
> basically capable of being verified (e.g., not a temp table).

I was using "downgrading" to mean downgrading from bt_index_parent_check() to bt_index_check() when pg_is_in_recovery() is true, but you've clarified that you're not requesting that downgrade, so I think we've now gotten past the last sticking point about that whole issue.

There are other sticking points that don't seem to be things you have taken a view on. Specifically, pg_amcheck complains if a relation pattern doesn't match anything, so that

pg_amcheck --table="*acountng*"

will complain if no tables match, giving the user the opportunity to notice that they spelled "accounting" wrong. If there happens to be a table named "xyzacountngo", and that matches, too bad. There isn't any way pg_amcheck can be responsible for that. But if there is a temporary table named "xyzacountngo" and that gets skipped because it's a temp table, I don't know what feedback the user should get. That's a thorny user interfaces question, not a corruption checking question, and I don't think you need to weigh in unless you want to. I'll most likely go with whatever is the simplest to code and/or most similar to what is currently in the tree, because I don't see any knock-down arguments one way or the other.

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