Re: Setting pd_lower in GIN metapage

On Mon, Jun 19, 2017 at 11:37 AM, Amit Langote
<Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> What are some arguments against setting pd_lower in the GIN metapage as
> follows?
>
> GinMetaPageData *metad = GinPageGetMeta(page);
>
> ((PageHeader) page)->pd_lower =
> ((char *) metad + sizeof(GinMetaPageData)) - (char *) page;
>
> I saw that _bt_initmetapage() does it, so was wondering why doesn't GIN.
>

Actually, hash index also has similar code (See _hash_init_metabuffer)
and I see no harm in doing this at similar other places. This helps
in compressing the hole in metapage during WAL writing. I think that
in itself might not be an argument in favor of doing this because
there is only one meta page for index and saving ~7K WAL is not huge
but OTOH I don't see a reason for not doing it.

> How about porting such a change to the back-branches if we do this at all?
> I couldn't find any discussion in the archives about this. I read in
> comments that server versions older than 9.4 didn't set pd_lower correctly
> in any of GIN index pages, so relying on pd_lower value of GIN pages is
> unreliable in general.
>
> The reason I'm asking is that a certain backup tool relies on pd_lower
> values of data pages (disk blocks in relation files that are known to have
> a valid PageHeaderData) to be correct to discard the portion of every page
> that supposedly does not contain any useful information. The assumption
> doesn't hold in the case of GIN metapage, so any GIN indexes contain
> corrupted metapage after recovery (metadata overwritten with zeros).
>

Why can't you do a special check for metapage identification? It
should be easy to add such a check. I have seen one of such tools
(pg_filedump) has similar check to skip metapage in certain code
paths.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com