Re: pgstattuple "unexpected zero page" for gist and hash indexes

Поиск
Список
Период
Сортировка
От Dilip Kumar
Тема Re: pgstattuple "unexpected zero page" for gist and hash indexes
Дата
Msg-id CAFiTN-sjz2+7FpXheAQag81y_3FsaQjwzaTTTUi6kmVhwqQh5A@mail.gmail.com
обсуждение исходный текст
Ответ на pgstattuple "unexpected zero page" for gist and hash indexes  (Nitin Motiani <nitinmotiani@google.com>)
Ответы Re: pgstattuple "unexpected zero page" for gist and hash indexes
Список pgsql-hackers
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



В списке pgsql-hackers по дате отправления: