On 2017/06/19 22:59, Amit Kapila wrote: > On Mon, Jun 19, 2017 at 11:37 AM, Amit Langote > <langote_amit...@lab.ntt.co.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.
I agree. Will write a patch. >> 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. I tried to add a metapage check, but getting such code to compile requires it to include headers like gin_private.h (in PG versions < 10), which in turn includes other headers that are forbidden to be included in what's supposed to be FRONTEND code. (thread at [1] seems relevant in this regard.) Another way to fix this might be to copy the GinMetaPageData struct definition and a few other symbols into the tool's code and make necessary checks using the same, instead of including gin_private.h. Thanks, Amit [1] https://www.postgresql.org/message-id/CA%2BTgmoZ%3DF%3DGkxV0YEv-A8tb%2BAEGy_Qa7GSiJ8deBKFATnzfEug%40mail.gmail.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers