Re: [HACKERS] Setting pd_lower in GIN metapage
От | Amit Langote |
---|---|
Тема | Re: [HACKERS] Setting pd_lower in GIN metapage |
Дата | |
Msg-id | c3ab4434-3fde-2c44-ad80-283257993869@lab.ntt.co.jp обсуждение исходный текст |
Ответ на | Re: [HACKERS] Setting pd_lower in GIN metapage (Michael Paquier <michael.paquier@gmail.com>) |
Ответы |
Re: [HACKERS] Setting pd_lower in GIN metapage
(Amit Kapila <amit.kapila16@gmail.com>)
Re: [HACKERS] Setting pd_lower in GIN metapage (Michael Paquier <michael.paquier@gmail.com>) |
Список | pgsql-hackers |
Hi. Trying to catch up. On 2017/09/25 13:43, Michael Paquier wrote: > On Sun, Sep 24, 2017 at 2:25 PM, Amit Kapila <amit.kapila16@gmail.com> wrote: >> Added and updated the comments for both btree and hash index patches. > > I don't have real complaints about this patch, this looks fine to me. > > + * Currently, the advantage of setting pd_lower is in limited cases like > + * during wal_consistency_checking or while logging for unlogged relation > + * as for all other purposes, we initialize the metapage. Note, it also > + * helps in page masking by allowing to mask unused space. > I would have reworked this comment a bit, say like that: > Setting pd_lower is useful for two cases which make use of WAL > compressibility even if the meta page is initialized at replay: > - Logging of init forks for unlogged relations. > - wal_consistency_checking logs extra full-page writes, and this > allows masking of the unused space of the page. > > Now I often get complains that I suck at this exercise ;) So, I think I understand the discussion so far and the arguments about what we should write to explain why we're setting pd_lower to the correct value. Just to remind, I didn't actually start this thread [1] to address the issue that the FPWs of meta pages written to WAL are not getting compressed. An external backup tool relies on pd_lower to give the correct starting offset of the hole to compress, provided the page's other fields suggest it has the standard page layout. Since we discovered that pd_lower is not set correctly in gin, brin, and spgist meta pages, I created patches to do the same. You (Michael) pointed out that that actually helps compress their FPW images in WAL as well, so we began considering that. Also, you pointed out that WAL checking code masks pages based on the respective masking functions' assumptions about the page's layout properties, which the original patches forgot to consider. So, we updated the patches to change the respective masking functions to mask meta pages as pages with standard page layout, too. But as Tom pointed out [2], WAL compressibility benefit cannot be obtained unless we change how the meta page is passed to xlog.c to be written to WAL. So, we found out all the places that write/register the meta page to WAL and changed the respective XLogRegisterBuffer calls to include the REGBUG_STANDARD flag. Some of these calls already passed REGBUF_WILL_INIT, which would result in no FPW image to be written to the WAL and so there would be no question of compressibility. But, passing REGBUF_STANDARD would still help the case where WAL checking is enabled, because FPW image *is* written in that case. So, ISTM, comments that the patches add should all say that setting the meta pages' pd_lower to the correct value helps to pass those pages to xlog.c as compressible standard layout pages, regardless of whether they are actually passed that way. Even if the patches do take care of the latter as well. Did I miss something? Looking at Amit K's updated patches for btree and hash, it seems that he updated the comment to say that setting pd_lower to the correct value is *essential*, because those pages are passed as REGBUF_STANDARD pages and hence will be compressed. That seems to contradict what I wrote above, but I think that's not wrong, too. So, I updated the gin, brin, and spgist patches to say the same, viz. the following: /* * Set pd_lower just past the end of the metadata. This is essential, * because without doing so, metadata will be lost if xlog.c compresses * the page. */ Attached updated patches. Thanks, Amit [1] https://www.postgresql.org/message-id/0d273805-0e9e-ec1a-cb84-d4da400b8f85%40lab.ntt.co.jp [2] https://www.postgresql.org/message-id/22268.1504815869%40sss.pgh.pa.us -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Вложения
В списке pgsql-hackers по дате отправления: