On Wed, Dec 17, 2014 at 11:33 PM, Rahila Syed <rahilasye...@gmail.com> wrote:
> I had a look at code. I have few minor points, > Thanks! + bkpb.fork_flags |= BKPBLOCK_HAS_IMAGE; > + > + if (is_compressed) > { > - rdt_datas_last->data = page; > - rdt_datas_last->len = BLCKSZ; > + /* compressed block information */ > + bimg.length = compress_len; > + bimg.extra_data = hole_offset; > + bimg.extra_data |= XLR_BLCK_COMPRESSED_MASK; > > For consistency with the existing code , how about renaming the macro > XLR_BLCK_COMPRESSED_MASK as BKPBLOCK_HAS_COMPRESSED_IMAGE on the lines of > BKPBLOCK_HAS_IMAGE. > OK, why not... > + blk->hole_offset = extra_data & ~XLR_BLCK_COMPRESSED_MASK; > Here , I think that having the mask as BKPBLOCK_HOLE_OFFSET_MASK will be > more indicative of the fact that lower 15 bits of extra_data field > comprises of hole_offset value. This suggestion is also just to achieve > consistency with the existing BKPBLOCK_FORK_MASK for fork_flags field. > Yeah that seems clearer, let's define it as ~XLR_BLCK_COMPRESSED_MASK though. And comment typo > + * First try to compress block, filling in the page hole with > zeros > + * to improve the compression of the whole. If the block is > considered > + * as incompressible, complete the block header information as > if > + * nothing happened. > > As hole is no longer being compressed, this needs to be changed. > Fixed. As well as an additional comment block down. A couple of things noticed on the fly: - Fixed pg_xlogdump being not completely correct to report the FPW information - A couple of typos and malformed sentences fixed - Added an assertion to check that the hole offset value does not the bit used for compression status - Reworked docs, mentioning as well that wal_compression is off by default. - Removed stuff in pg_controldata and XLOG_PARAMETER_CHANGE (mentioned by Fujii-san) Regards, -- Michael
20141218_fpw_compression_v8.tar.gz
Description: GNU Zip compressed data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers