Hello, Thank you for review.
>1) I don't think it's a good idea to put the full page write compression into struct XLogRecord. Full page write compression information can be stored in varlena struct of compressed blocks as done for toast data in pluggable compression support patch. If I understand correctly, it can be done similar to the manner in which compressed Datum is modified to contain information about compression algorithm in pluggable compression support patch. >2) You've essentially removed a lot of checks about the validity of bkp blocks in xlogreader. I don't think that's acceptable To ensure this, the raw size stored in first four byte of compressed datum can be used to perform error checking for backup blocks Currently, the error checking for size of backup blocks happens individually for each block. If backup blocks are compressed together , it can happen once for the entire set of backup blocks in a WAL record. The total raw size of compressed blocks can be checked against the total size stored in WAL record header. >3) You have both FullPageWritesStr() and full_page_writes_str(). full_page_writes_str() is true/false version of FullPageWritesStr macro. It is implemented for backward compatibility with pg_xlogdump >4)I don't like FullPageWritesIsNeeded(). For one it, at least to me, sounds grammatically wrong. More importantly when reading it I'm thinking of it being about the LSN check. How about instead directly checking whatever != FULL_PAGE_WRITES_OFF? I will modify this. >5) CompressBackupBlockPagesAlloc is declared static but not defined as such. >7) Unless I miss something CompressBackupBlock should be plural, right? ATM it compresses all the blocks? I will correct these. >6)You call CompressBackupBlockPagesAlloc() from two places. Neither is > IIRC within a critical section. So you imo should remove the outOfMem > handling and revert to palloc() instead of using malloc directly. Yes neither is in critical section. outOfMem handling is done in order to proceed without compression of FPW in case sufficient memory is not available for compression. Thank you, Rahila Syed -- View this message in context: http://postgresql.1045698.n5.nabble.com/Compression-of-full-page-writes-tp5769039p5822391.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
