On Thu, Apr 19, 2018 at 9:42 AM, Teodor Sigaev <teodor@sigaev.ru> wrote:
> But some later, in _bt_unlink_halfdead_page() we check ItemPointer high key
> with ItemPointerIsValid macro - and it returns false, because offset is
> actually InvalidOffsetNumber - i.e. 0 which was set by BTreeTupleSetNAtts.
> And some wrong decisions are follows, I didn't look at that.
The problem here is that multi-level page deletion incorrectly thinks
that it's safe to fully delete a half-dead leaf page. Of course, this
is due to the issue with the representation of high keys that you
identified. IOW, VACUUM incorrect believes that fully deleting the
leaf page will leave no "dangling downlink" one level up, but, in
fact, there is one such dangling downlink. This could eventually lead
to wrong answers to queries, since the deleted page will eventually be
fully recycled.
I refined the amcheck enhancement quite a bit today. It will not just
check that a downlink is not missing; It will also confirm that it
wasn't a legitimately interrupted multi-level deletion, by descending
to the leaf page to match the leaf high key pointer to the top most
parent, which should be the target page (the page that lacked a
downlink according to the new Bloom filter). We need to worry about
multi-level deletions that are interrupted by an error or a hard
crash, which presents a legitimate case where there'll be no downlink
for an internal page in its parent. VACUUM is okay with that, so we
must be too.
I'll finish it off early next week as planned. It should
comprehensively cover issues with multi-level page deletion, including
cases where you have this kind of index corruption and page recycling
later makes it much worse. So, it won't just cover the exact test case
from Michael, which produced corruption that can be detected by
throwing an error when a downlink leads to a fully deleted page
(though only when amcheck has a ShareLock, of course).
Note that the problem here is best understood as a problem around the
presence of a dangling downlink, as oppose to a problem around the
absence of a needed downlink. There is an absent downlink, but that's
beside the point, and in any case missing a downlink is not
*inherently* wrong (as I said, it's not inherently wrong because of
the issue of interrupted multi-level page deletes).
--
Peter Geoghegan