On Mon, Sep 25, 2017 at 3:48 PM, Amit Langote
<langote_amit...@lab.ntt.co.jp> wrote:
> Trying to catch up.

Glad to see you back.

> 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.

Thanks for summarizing all that happened.

> 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?

Not that I think of.

        Buffer      metabuffer;
+       Page        metapage;
        SpGistMetaPageData *metadata;

        metabuffer = ReadBuffer(index, SPGIST_METAPAGE_BLKNO);
+       metapage = BufferGetPage(metabuffer);
No need to define metapage here and to call BufferGetPage() as long as
the lock on the buffer is not taken.

Except that small thing, the patches do their duty.

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to