On Thu, Apr 19, 2018 at 9:42 AM, Teodor Sigaev <teo...@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