Re: room for improvement in amcheck btree checking?

On Tue, Dec 1, 2020 at 12:39 PM Mark Dilger
<mark.dilger@enterprisedb.com> wrote:
> I found it surprising that even when precisely zero of the tids in the index exist in the table the index checks all
comeback clean.  The heapallindexed check is technically running as advertised, checking that all of the zero tuples in
theheap are present in the index.  That is a pretty useless check under this condition, though.  Is a "indexallheaped"
option(by some less crazy name) needed? 
>
> Users might also run into this problem when a heap relation file gets erroneously shortened by some number of blocks
butnot fully truncated, or perhaps with torn page writes. 

It would probably be possible to detect this exact condition (though
not other variants of the more general problem) relatively easily.
Perhaps by doing something with RelationOpenSmgr() with the heap
relation, while applying a little knowledge of what must be true about
the heap relation given what is known about the index relation. (I'm
not 100% sure that that's possible, but offhand it seems like it
probably is.)

See also: commit 6754fe65a4c, which tightened things up in this area
for the index relation itself.

> Have you already considered and rejected a "indexallheaped" type check?

Actually, I pointed out that we should do something along these lines
very recently:

https://postgr.es/m/CAH2-Wz=dy--FG5iJ0kPcQumS0W5g+xQED3t-7HE+UqAK_hmLTw@mail.gmail.com

I'd be happy to see you pursue it.

> Background
> -------
>
> I have up until recently been focused on corruption caused by twiddling the bits within heap and index relation
pages,but real-world user error, file system error, and perhaps race conditions in the core postgresql code seem at
leastas likely to result in missing or incorrect versions of blocks of relation files rather than individual bytes
withinthose blocks being wrong.  Per our discussions in [3], not all corruptions that can be created under laboratory
conditionsare equally likely to occur in the wild, and it may be reasonable to only harden the amcheck code against
corruptionsthat are more likely to happen in actual practice. 

Right. I also like to harden amcheck in ways that happen to be easy,
especially when it seems like it might kind of work as a broad
defense that doesn't consider any particular scenario. For example,
hardening to make sure the an index tuple's lp_len matches
IndexTupleSize() for the tuple proper (this also kind of works as
storage format documentation). I am concerned about both costs and
benefits.

FWIW, this is a perfect example of the kind of hardening that makes
sense to me. Clearly this kind of failure is a reasonably plausible
scenario (probably even a known real world scenario with catalog
corruption), and clearly we could do something about it that's pretty
simple and obviously low risk. It easily meets the standard that I
might apply here.

--
Peter Geoghegan