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

On Tue, Oct 28, 2014 at 4:54 PM, Syed, Rahila <Rahila(dot)Syed(at)nttdata(dot)com> wrote:
> 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.

Don't we need to do that?

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

You don't need to make the processes except the startup process allocate
the memory for uncompressedPages when fpw=on. Only the startup process
uses it for the WAL decompression.

BTW, what happens if the memory allocation for uncompressedPages for
the recovery fails? Which would prevent the recovery at all, so PANIC should
happen in that case?

Regards,

--
Fujii Masao