On 25/11/2025 22:51, Peter Geoghegan wrote:
On Tue, Nov 25, 2025 at 3:03 PM Heikki Linnakangas <[email protected]> wrote:
To fix this, I guess we need to teach bt_index_parent_check() about
half-dead pages. Anyone volunteer to write that patch?
It's not like bt_index_parent_check doesn't generally know about them.
For example, bt_downlink_missing_check goes to great lengths to
distinguish between legitimate "missing" downlinks caused by an
interrupted page deletion, and real missing downlinks caused by
corruption.
The problem we're seeing here seems likely limited to code added by
commit d114cc53, which enhanced bt_index_parent_check by adding the
new bt_child_highkey_check check. bt_child_highkey_check actually
reuses bt_downlink_missing_check (which deals with half-dead pages
correctly), but still isn't careful enough about half-dead pages. This
is kind of surprising, since it *does* account for incomplete splits,
which are similar.
+1. It took me a moment to understand bt_downlink_missing_check(). The
comments there talk about interrupted page deletions and incomplete
splits, but it's actually never called for half-dead pages. The caller
checks !P_IGNORE(opaque), and P_IGNORE means BTP_DELETED | BTP_HALF_DEAD.
I don't think BTP_DELETED pages should be reachable here at all. So
perhaps we should specifically check BTP_DELETED and give an ERROR on
those. And for clarity, perhaps move the check for BTP_HALF_DEAD into
bt_downlink_missing_check(), alongside the incomplete-split check, so
that both cases would be checked at the same place. Just for clarity,
it's not wrong as it is.
In short, I think that we need to track something like
BtreeCheckState.previncompletesplit, but for half-dead pages. And then
actually use that within bt_child_highkey_check, to avoid spurious
false-positive reports of corruption.
I think it's even simpler than that, and we can just do this:
--- a/contrib/amcheck/verify_nbtree.c
+++ b/contrib/amcheck/verify_nbtree.c
@@ -2268,7 +2268,7 @@ bt_child_highkey_check(BtreeCheckState *state,
* If we visit page with high key, check that it is equal to the
* target key next to corresponding downlink.
*/
- if (!rightsplit && !P_RIGHTMOST(opaque))
+ if (!rightsplit && !P_RIGHTMOST(opaque) &&
!P_ISHALFDEAD(opaque))
{
BTPageOpaque topaque;
IndexTuple highkey;
Both BTP_HALF_DEAD and BTP_INCOMPLETE_SPLIT indicate that a downlink is
missing, but they work slightly differently. If a page is marked as
BTP_INCOMPLETE_SPLIT, it means that its right sibling has no downlink,
but if a page is marked as BTP_HALF_DEAD, it means that the page itself
has no downlink.
- Heikki
