>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 sufficient memory 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.paqu...@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.s...@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 storing raw_length of the compressed > block. > In order to achieve compression while accounting for these two additional > bytes, we must ensure that compressed length is 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 to store 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 block header. + * 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.
Support-compression-for-full-page-writes-in-WAL_v17.patch
Description: Support-compression-for-full-page-writes-in-WAL_v17.patch
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
