Re: [REVIEW] Re: Compression of full-page-writes

Hello Fujii-san,

Thank you for your comments.

>The patch isn't applied to the master cleanly.
>The compilation of the document failed with the following error message.
>openjade:config.sgml:2188:12:E: end tag for element "TERM" which is not open
>make[3]: *** [HTML.index] Error 1
>xlog.c:930: warning: ISO C90 forbids mixed declarations and code
>xlogreader.c:744: warning: ISO C90 forbids mixed declarations and code
>xlogreader.c:744: warning: ISO C90 forbids mixed declarations and code

Please find attached patch with these rectified.

>Only backend calls CompressBackupBlocksPagesAlloc when SIGHUP is sent.
>Why does only backend need to do that? What about other processes which can write FPW, e.g., autovacuum?
I had overlooked this. I will correct it.

>Do we release the buffers for compressed data when fpw is changed from "compress" to "on"?
The current code does not do this.

>The memory is always (i.e., even when fpw=on) allocated to uncompressedPages, but not to compressedPages. Why? I guess
thatthe test of fpw needs to be there 
uncompressedPages is also used to store the decompression output at the time of recovery. Hence, memory for
uncompressedPagesneeds to be allocated even if fpw=on which is not the case for compressedPages. 

Thank you,
Rahila Syed

-----Original Message-----
From: Fujii Masao [mailto:masao.fujii@gmail.com]
Sent: Monday, October 27, 2014 6:50 PM
To: Rahila Syed
Cc: Andres Freund; Syed, Rahila; Alvaro Herrera; Rahila Syed; PostgreSQL-development; Tom Lane
Subject: Re: [HACKERS] [REVIEW] Re: Compression of full-page-writes

On Fri, Oct 17, 2014 at 1:52 PM, Rahila Syed <rahilasyed90@gmail.com> wrote:
> Hello,
>
> Please find the updated patch attached.

Thanks for updating the patch! Here are the comments.

The patch isn't applied to the master cleanly.

I got the following compiler warnings.

xlog.c:930: warning: ISO C90 forbids mixed declarations and code
xlogreader.c:744: warning: ISO C90 forbids mixed declarations and code
xlogreader.c:744: warning: ISO C90 forbids mixed declarations and code

The compilation of the document failed with the following error message.

openjade:config.sgml:2188:12:E: end tag for element "TERM" which is not open
make[3]: *** [HTML.index] Error 1

Only backend calls CompressBackupBlocksPagesAlloc when SIGHUP is sent.
Why does only backend need to do that? What about other processes which can write FPW, e.g., autovacuum?

Do we release the buffers for compressed data when fpw is changed from "compress" to "on"?

+    if (uncompressedPages == NULL)
+    {
+        uncompressedPages = (char *)malloc(XLR_TOTAL_BLCKSZ);
+        if (uncompressedPages == NULL)
+            outOfMem = 1;
+    }

The memory is always (i.e., even when fpw=on) allocated to uncompressedPages, but not to compressedPages. Why? I guess
thatthe test of fpw needs to be there. 

Regards,

--
Fujii Masao

______________________________________________________________________
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.