Re: [REVIEW] Re: Compression of full-page-writes - Mailing list pgsql-hackers
From | Syed, Rahila |
---|---|
Subject | Re: [REVIEW] Re: Compression of full-page-writes |
Date | |
Msg-id | C3C878A2070C994B9AE61077D46C3846589AC8E5@MAIL703.KDS.KEANE.COM Whole thread Raw |
In response to | Re: [REVIEW] Re: Compression of full-page-writes (Michael Paquier <michael.paquier@gmail.com>) |
Responses |
Re: [REVIEW] Re: Compression of full-page-writes
|
List | pgsql-hackers |
>In any case, those things have been introduced by what I did in previous versions... And attached is a new patch. Thank you for feedback. > /* allocate scratch buffer used for compression of block images */ >+ if (compression_scratch == NULL) >+ compression_scratch = MemoryContextAllocZero(xloginsert_cxt, >+ BLCKSZ); >} The compression patch can use the latest interface MemoryContextAllocExtended to proceed without compression when sufficientmemory is not available for scratch buffer. The attached patch introduces OutOfMem flag which is set on when MemoryContextAllocExtended returns NULL . Thank you, Rahila Syed -----Original Message----- From: Michael Paquier [mailto:michael.paquier@gmail.com] Sent: Friday, February 06, 2015 12:46 AM To: Syed, Rahila Cc: PostgreSQL mailing lists Subject: Re: [HACKERS] [REVIEW] Re: Compression of full-page-writes On Thu, Feb 5, 2015 at 11:06 PM, Syed, Rahila <Rahila.Syed@nttdata.com> wrote: >>/* >>+ * We recheck the actual size even if pglz_compress() report success, >>+ * because it might be satisfied with having saved as little as one byte >>+ * in the compressed data. >>+ */ >>+ *len = (uint16) compressed_len; >>+ if (*len >= orig_len - 1) >>+ return false; >>+ return true; >>+} > > As per latest code ,when compression is 'on' we introduce additional 2 bytes in the header of each block image for storingraw_length of the compressed block. > In order to achieve compression while accounting for these two additional bytes, we must ensure that compressed lengthis less than original length - 2. > So , IIUC the above condition should rather be > > If (*len >= orig_len -2 ) > return false; > return true; > The attached patch contains this. It also has a cosmetic change- renaming compressBuf to uncompressBuf as it is used tostore uncompressed page. Agreed on both things. Just looking at your latest patch after some time to let it cool down, I noticed a couple of things. #define MaxSizeOfXLogRecordBlockHeader \ (SizeOfXLogRecordBlockHeader + \ - SizeOfXLogRecordBlockImageHeader + \ + SizeOfXLogRecordBlockImageHeader, \ + SizeOfXLogRecordBlockImageCompressionInfo + \ There is a comma here instead of a sum sign. We should really sum up all those sizes to evaluate the maximum size of a blockheader. + * Permanently allocate readBuf uncompressBuf. We do it this way, + * rather than just making a static array, for two reasons: This comment is just but weird, "readBuf AND uncompressBuf" is more appropriate. + * We recheck the actual size even if pglz_compress() report success, + * because it might be satisfied with having saved as little as one byte + * in the compressed data. We add two bytes to store raw_length with the + * compressed image. So for compression to be effective compressed_len should + * be atleast < orig_len - 2 This comment block should be reworked, and misses a dot at its end. I rewrote it like that, hopefully that's clearer: + /* + * We recheck the actual size even if pglz_compress() reports success and see + * if at least 2 bytes of length have been saved, as this corresponds to the + * additional amount of data stored in WAL record for a compressed block + * via raw_length. + */ In any case, those things have been introduced by what I did in previous versions... And attached is a new patch. -- Michael ______________________________________________________________________ Disclaimer: This email and any attachments are sent in strictest confidence for the sole use of the addressee and may contain legally privileged, confidential, and proprietary data. If you are not the intended recipient, please advise the sender by replying promptly to this email and then delete and destroy this email and any attachments without any further use, copying or forwarding.
Attachment
pgsql-hackers by date: