Re: pgstattuple "unexpected zero page" for gist and hash indexes
Dilip Kumar <dilipbalaut@gmail.com>
From: Dilip Kumar <dilipbalaut@gmail.com>
To: Nitin Motiani <nitinmotiani@google.com>
Cc: pgsql-hackers@postgresql.org
Date: 2025-09-29T03:33:13Z
Lists: pgsql-hackers
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
pgstattuple: Improve reports generated for indexes (hash, gist, btree)
- c207bf473ee9 13.23 landed
- ee2dbcad02c7 14.20 landed
- 49b5f0b53c63 15.15 landed
- c0f9fe877e23 16.11 landed
- 036decbba2af 17.7 landed
- fc295beb7b74 18.1 landed
- 684a745f5505 19 (unreleased) landed
On Tue, Sep 16, 2025 at 1:06 PM Nitin Motiani <nitinmotiani@google.com> wrote:
>
> For hash indexes, this error comes from pgstat_hash_page() ->
> _hash_getbuf_with_strategy() -> _hash_checkpage() for a zero page.
> Similarly gistcheckpage() is the cause for gist indexes.
>
> Since after a crash recovery a zero page is normal (as long as not
> referenced from the rest of the index), emitting
> ERRCODE_INDEX_CORRUPTED is a false alarm.
I agree this seems like a false alarm, and at least its inconsistent
with the btree.
> To fix this, we propose that we remove the checks from
> pgstat_hash_page() and pgstat_gist_page(). This is something which
> pgstat_btree_page() already does. It uses a lower-level function
> ReadBufferExtended() and avoids using _bt_getbuf() which would check
> for the "unexpected zero page".
>
> I'm attaching a patch for the above which does the following :
>
> 1. Replaces _hash_getbuf_with_strategy() with ReadBufferExtended() in
> pgstat_hash_page(). Then for non-PageIsNew() pages, we explicitly
> check the PageGetSpecialSize().
>
> 2. Similarly gistcheckpage() is removed in pgstat_gist_page(). And for
> non-PageIsNew(), we explicitly check the PageGetSpecialSize() before
> checking if it's a leaf page.
>
> I confirmed that this was working by running the above mentioned
> simulation steps along with the patch.
Yeah that seems fine to me.
> Note that _hash_getbuf_with_strategy() also checks if blkno is P_NEW.
> But that check seems unnecessary here as P_NEW is the same as
> InvalidBlockNo and pgstat_hash_page() is called by pgstat_index() for
> the block nos starting from HASH_METAPAGE + 1 and remains <
> RelationGetNumberOfBlocks().
Right, we don't need that check here.
I observed that pgstat_btree_page() incorporates the count of new
pages into its free space calculation[1]. Does it make sense to do the
same for hash and gist as well as we are leaning towards making these
consistent.
[1]
if (PageIsNew(page))
{
/* fully empty page */
stat->free_space += BLCKSZ;
}
--
Regards,
Dilip Kumar
Google