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

> On Oct 6, 2021, at 3:20 PM, Peter Geoghegan <pg@bowt.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
slideupthread as a convenient short-hand, but I think it doesn't help.  For pg_amcheck to take any view whatsoever on
whethera 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
thatfact and eventually exits with a non-zero exit code.  The error might have been something about the command exiting
dueto the crash of another backend, or to do with a deadlock against some other process, or whatever, and pg_amcheck
hasno 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
talkingabout whether pg_amcheck issues all the commands that it is supposed to issue.  If I work for Daddy Warbucks and
hegives me 30 classic cars to take to 10 different mechanics, I can do that job perfectly even if the mechanics do less
thanperfect work.  If I leave three cars in the driveway, that's on me.  Likewise, it's not on pg_amcheck if the
checkingfunctions can't do perfect work, but it is on pg_amcheck if it doesn't issue all the expected commands.  But
lateron 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
orhow 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
Idid have a problem with that upthread, but its been a while since I conceded that point so I'd rather not have to
argueit 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
thebackend 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
temporarytables 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
inrecovery. 
>
> 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_amcheckto filter out the --parent-check option when in recovery, but if you're not asking for that, then I might
haveenough 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()
istrue, but you've clarified that you're not requesting that downgrade, so I think we've now gotten past the last
stickingpoint 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
complainsif 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
therehappens to be a table named "xyzacountngo", and that matches, too bad.  There isn't any way pg_amcheck can be
responsiblefor 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
checkingquestion, and I don't think you need to weigh in unless you want to.  I'll most likely go with whatever is the
simplestto code and/or most similar to what is currently in the tree, because I don't see any knock-down arguments one
wayor the other. 

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