Greetings, * Kyotaro HORIGUCHI (horiguchi.kyot...@lab.ntt.co.jp) wrote: > At Wed, 28 Nov 2018 10:01:36 +0900, Michael Paquier <mich...@paquier.xyz> > wrote in <20181128010136.gu1...@paquier.xyz> > > On Tue, Nov 27, 2018 at 07:51:03PM -0500, Stephen Frost wrote: > > > That isn't at all what I got from that. > > > > > > A rewrite of this really should avoid talking about removing the hole as > > > if it's 'compression' because, first of all, it isn't, and second, now > > > that we have *actual* compression happening, it's terribly confusing to > > > talk about removing the hole using that terminology. > > > > You may want to be careful about other comments in xloginsert.c or such > > then. Removing a page hole is mentioned in a couple of places as a form > > of compression if I recall correctly. > > org> When wal_compression is enabled, a full page image which "hole" was > org> removed is additionally compressed using PGLZ compression algorithm. > > Even though it has an obvious mistake, I could read this as > full_page_image is always compressed after removing a hole in it > if any. (I'm not sure it makes correct sense, though.) I feel > hopelessly that the sentence structure model in my mind is > different from that of natives. I need to study harder, but..
Thanks to everyone for sharing their thoughts here, the goal is simply to try and have the comments as clear as we can for everyone. Please find attached a larger rewording of the comment in xlogrecord.h, and a minor change to xloginsert.c to clarify that we're removing a hole and not doing compression at that point. Other thoughts on this..? Thanks! Stephen
From febd0feac0a7e2cb216c7cae5af7a5c458d3e394 Mon Sep 17 00:00:00 2001 From: Stephen Frost <sfr...@snowman.net> Date: Tue, 4 Dec 2018 11:30:37 -0500 Subject: [PATCH] Cleanup comments in xlog compression Skipping over the "hole" in full page images in the XLOG code was described as being a form of compression, but this got a bit confusing since we now have PGLZ-based compression happening, so adjust the wording to discuss "removing" the "hole" and keeping the talk about compression to where we're talking about using PGLZ-based compression of the full page images. Discussion: https://postgr.es/m/20181127234341.gm3...@tamriel.snowman.net --- src/backend/access/transam/xloginsert.c | 2 +- src/include/access/xlogrecord.h | 13 ++++++------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c index 34d4db4297..034d5b3b62 100644 --- a/src/backend/access/transam/xloginsert.c +++ b/src/backend/access/transam/xloginsert.c @@ -605,7 +605,7 @@ XLogRecordAssemble(RmgrId rmid, uint8 info, } else { - /* No "hole" to compress out */ + /* No "hole" to remove */ bimg.hole_offset = 0; cbimg.hole_length = 0; } diff --git a/src/include/access/xlogrecord.h b/src/include/access/xlogrecord.h index 863781937e..070d6d7cf1 100644 --- a/src/include/access/xlogrecord.h +++ b/src/include/access/xlogrecord.h @@ -107,15 +107,14 @@ typedef struct XLogRecordBlockHeader * Additional header information when a full-page image is included * (i.e. when BKPBLOCK_HAS_IMAGE is set). * - * As a trivial form of data compression, the XLOG code is aware that - * PG data pages usually contain an unused "hole" in the middle, which - * contains only zero bytes. If the length of "hole" > 0 then we have removed - * such a "hole" from the stored data (and it's not counted in the - * XLOG record's CRC, either). Hence, the amount of block data actually + * The XLOG code is aware that PG data pages usually contain an unused "hole" + * in the middle, which contains only zero bytes. Since we know that the + * "hole" is all zeros, we remove it from the stored data (and it's not counted + * in the XLOG record's CRC, either). Hence, the amount of block data actually * present is BLCKSZ - the length of "hole" bytes. * - * When wal_compression is enabled, a full page image which "hole" was - * removed is additionally compressed using PGLZ compression algorithm. + * Additionally, when wal_compression is enabled, we will try to compress full + * page images using the PGLZ compression algorithm, after removing the "hole". * This can reduce the WAL volume, but at some extra cost of CPU spent * on the compression during WAL logging. In this case, since the "hole" * length cannot be calculated by subtracting the number of page image bytes -- 2.17.1
signature.asc
Description: PGP signature
