On Thu, Feb 12, 2015 at 8:08 PM, Syed, Rahila <rahila.s...@nttdata.com> wrote: > > > > Thank you for comments. Please find attached the updated patch. > > > > >This patch fails to compile: > >xlogreader.c:1049:46: error: extraneous ')' after condition, expected a statement > > blk->with_hole && blk->hole_offset <= 0)) > > This has been rectified. > > > > >Note as well that at least clang does not like much how the sanity check with with_hole are done. You should place parentheses around the '&&' expressions. Also, I would rather define with_hole == 0 or with_hole == 1 explicitly int those checks > > The expressions are modified accordingly. > > > > >There is a typo: > > >s/true,see/true, see/ > > >[nitpicky]Be as well aware of the 80-character limit per line that is usually normally by comment blocks.[/] > > > > Have corrected the typos and changed the comments as mentioned. Also , realigned certain lines to meet the 80-char limit.
Thanks for the updated patch. + /* leave if data cannot be compressed */ + if (compressed_len == 0) + return false; This should be < 0, pglz_compress returns -1 when compression fails. + if (pglz_decompress(block_image, bkpb->bkp_len, record->uncompressBuf, + bkpb->bkp_uncompress_len) == 0) Similarly, this should be < 0. Regarding the sanity checks that have been added recently. I think that they are useful but I am suspecting as well that only a check on the record CRC is done because that's reliable enough and not doing those checks accelerates a bit replay. So I am thinking that we should simply replace them by assertions. I have as well re-run my small test case, with the following results (scripts and results attached) =# select test, user_diff,system_diff, pg_size_pretty(pre_update - pre_insert), pg_size_pretty(post_update - pre_update) from results; test | user_diff | system_diff | pg_size_pretty | pg_size_pretty ---------+-----------+-------------+----------------+---------------- FPW on | 46.134564 | 0.823306 | 429 MB | 566 MB FPW on | 16.307575 | 0.798591 | 171 MB | 229 MB FPW on | 8.325136 | 0.848390 | 86 MB | 116 MB FPW off | 29.992383 | 1.100458 | 440 MB | 746 MB FPW off | 12.237578 | 1.027076 | 171 MB | 293 MB FPW off | 6.814926 | 0.931624 | 86 MB | 148 MB HEAD | 26.590816 | 1.159255 | 440 MB | 746 MB HEAD | 11.620359 | 0.990851 | 171 MB | 293 MB HEAD | 6.300401 | 0.904311 | 86 MB | 148 MB (9 rows) The level of compression reached is the same as previous mark, 566MB for the case of fillfactor=50 ( cab7npqsc97o-ue5paxfmukwcxe_jioyxo1m4a0pmnmyqane...@mail.gmail.com) with a similar CPU usage. Once we get those small issues fixes, I think that it is with having a committer look at this patch, presumably Fujii-san. Regards, -- Michael
compress_run.bash
Description: Binary data
results.sql
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
