On Wed, Dec 3, 2014 at 2:17 AM, Robert Haas <robertmh...@gmail.com> wrote: > On Wed, Nov 26, 2014 at 11:00 PM, Michael Paquier > <michael.paqu...@gmail.com> wrote: >> On Wed, Nov 26, 2014 at 8:27 PM, Syed, Rahila <rahila.s...@nttdata.com> >> wrote: >>> Don't we need to initialize doPageCompression similar to doPageWrites in >>> InitXLOGAccess? >> Yep, you're right. I missed this code path. >> >>> Also , in the earlier patches compression was set 'on' even when fpw GUC is >>> 'off'. This was to facilitate compression of FPW which are forcibly written >>> even when fpw GUC is turned off. >>> doPageCompression in this patch is set to true only if value of fpw GUC is >>> 'compress'. I think its better to compress forcibly written full page >>> writes. >> Meh? (stealing a famous quote). >> This is backward-incompatible in the fact that forcibly-written FPWs >> would be compressed all the time, even if FPW is set to off. The >> documentation of the previous patches also mentioned that images are >> compressed only if this parameter value is switched to compress. > > If we have a separate GUC to determine whether to do compression of > full page writes, then it seems like that parameter ought to apply > regardless of WHY we are doing full page writes, which might be either > that full_pages_writes=on in general, or that we've temporarily turned > them on for the duration of a full backup.
In the latest versions of the patch, control of compression is done within full_page_writes by assigning a new value 'compress'. Something that I am scared of is that if we enforce compression when full_page_writes is off for forcibly-written pages and if a bug shows up in the compression/decompression algorithm at some point (that's unlikely to happen as this has been used for years with toast but let's say "if"), we may corrupt a lot of backups. Hence why not simply having a new GUC parameter to fully control it. First versions of the patch did that but ISTM that it is better than enforcing the use of a new feature for our user base. Now, something that has not been mentioned on this thread is to make compression the default behavior in all cases so as we won't even have to use a GUC parameter. We are usually conservative about changing default behaviors so I don't really think that's the way to go. Just mentioning the possibility. Regards, -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers