Обсуждение: pgsql: Add GUC to enable compression of full page images stored in WAL.
Add GUC to enable compression of full page images stored in WAL. When newly-added GUC parameter, wal_compression, is on, the PostgreSQL server compresses a full page image written to WAL when full_page_writes is on or during a base backup. A compressed page image will be decompressed during WAL replay. Turning this parameter on can reduce the WAL volume without increasing the risk of unrecoverable data corruption, but at the cost of some extra CPU spent on the compression during WAL logging and on the decompression during WAL replay. This commit changes the WAL format (so bumping WAL version number) so that the one-byte flag indicating whether a full page image is compressed or not is included in its header information. This means that the commit increases the WAL volume one-byte per a full page image even if WAL compression is not used at all. We can save that one-byte by borrowing one-bit from the existing field like hole_offset in the header and using it as the flag, for example. But which would reduce the code readability and the extensibility of the feature. Per discussion, it's not worth paying those prices to save only one-byte, so we decided to add the one-byte flag to the header. This commit doesn't introduce any new compression algorithm like lz4. Currently a full page image is compressed using the existing PGLZ algorithm. Per discussion, we decided to use it at least in the first version of the feature because there were no performance reports showing that its compression ratio is unacceptably lower than that of other algorithm. Of course, in the future, it's worth considering the support of other compression algorithm for the better compression. Rahila Syed and Michael Paquier, reviewed in various versions by myself, Andres Freund, Robert Haas, Abhijit Menon-Sen and many others. Branch ------ master Details ------- http://git.postgresql.org/pg/commitdiff/57aa5b2bb11a4dbfdfc0f92370e0742ae5aa367b Modified Files -------------- contrib/pg_xlogdump/pg_xlogdump.c | 28 +++-- doc/src/sgml/config.sgml | 24 +++++ src/backend/access/transam/xlog.c | 1 + src/backend/access/transam/xloginsert.c | 141 +++++++++++++++++++++---- src/backend/access/transam/xlogreader.c | 101 ++++++++++++++++-- src/backend/utils/misc/guc.c | 10 ++ src/backend/utils/misc/postgresql.conf.sample | 1 + src/include/access/xlog.h | 1 + src/include/access/xlog_internal.h | 2 +- src/include/access/xlogreader.h | 2 + src/include/access/xlogrecord.h | 48 ++++++++- 11 files changed, 320 insertions(+), 39 deletions(-)
Hi,
On 2015-03-11 06:54:16 +0000, Fujii Masao wrote:
> Add GUC to enable compression of full page images stored in WAL.
This triggers a couple warnings here (gcc 4.9 and 5):
/home/andres/src/postgresql/src/backend/access/transam/xloginsert.c: In function ‘XLogInsert’:
/home/andres/src/postgresql/src/backend/access/transam/xloginsert.c:670:5: warning: ‘cbimg.hole_length’ may be used
uninitializedin this function [-Wmaybe-uninitialized]
memcpy(scratch, &cbimg,
^
/home/andres/src/postgresql/src/backend/access/transam/xloginsert.c:494:33: note: ‘cbimg.hole_length’ was declared here
XLogRecordBlockCompressHeader cbimg;
^
/home/andres/src/postgresql/src/backend/access/transam/xloginsert.c:668:20: warning: ‘hole_length’ may be used
uninitializedin this function [-Wmaybe-uninitialized]
if (hole_length != 0 && is_compressed)
^
/home/andres/src/postgresql/src/backend/access/transam/xloginsert.c:497:10: note: ‘hole_length’ was declared here
uint16 hole_length;
I've not checked whether they're spurious or not.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Re: pgsql: Add GUC to enable compression of full page images stored in WAL.
От
Michael Paquier
Дата:
On Wed, Mar 11, 2015 at 11:18 PM, Andres Freund <andres@2ndquadrant.com> wrote: > On 2015-03-11 06:54:16 +0000, Fujii Masao wrote: >> Add GUC to enable compression of full page images stored in WAL. > > This triggers a couple warnings here (gcc 4.9 and 5): > > /home/andres/src/postgresql/src/backend/access/transam/xloginsert.c: In function 'XLogInsert': > /home/andres/src/postgresql/src/backend/access/transam/xloginsert.c:670:5: warning: 'cbimg.hole_length' may be used uninitializedin this function [-Wmaybe-uninitialized] > memcpy(scratch, &cbimg, > ^ > /home/andres/src/postgresql/src/backend/access/transam/xloginsert.c:494:33: note: 'cbimg.hole_length' was declared here > XLogRecordBlockCompressHeader cbimg; > ^ > /home/andres/src/postgresql/src/backend/access/transam/xloginsert.c:668:20: warning: 'hole_length' may be used uninitializedin this function [-Wmaybe-uninitialized] > if (hole_length != 0 && is_compressed) > ^ > /home/andres/src/postgresql/src/backend/access/transam/xloginsert.c:497:10: note: 'hole_length' was declared here > uint16 hole_length; > > I've not checked whether they're spurious or not. That's a little bit surprising, hole_offset is set and used only when needs_backup == true, same with cbigm.hole_offset when (needs_backup && is_compressed && hole_offset != 0). -- Michael
On Wed, Mar 11, 2015 at 11:30 PM, Michael Paquier <michael.paquier@gmail.com> wrote: > On Wed, Mar 11, 2015 at 11:18 PM, Andres Freund <andres@2ndquadrant.com> wrote: >> On 2015-03-11 06:54:16 +0000, Fujii Masao wrote: >>> Add GUC to enable compression of full page images stored in WAL. >> >> This triggers a couple warnings here (gcc 4.9 and 5): >> >> /home/andres/src/postgresql/src/backend/access/transam/xloginsert.c: In function 'XLogInsert': >> /home/andres/src/postgresql/src/backend/access/transam/xloginsert.c:670:5: warning: 'cbimg.hole_length' may be used uninitializedin this function [-Wmaybe-uninitialized] >> memcpy(scratch, &cbimg, >> ^ >> /home/andres/src/postgresql/src/backend/access/transam/xloginsert.c:494:33: note: 'cbimg.hole_length' was declared here >> XLogRecordBlockCompressHeader cbimg; >> ^ >> /home/andres/src/postgresql/src/backend/access/transam/xloginsert.c:668:20: warning: 'hole_length' may be used uninitializedin this function [-Wmaybe-uninitialized] >> if (hole_length != 0 && is_compressed) >> ^ >> /home/andres/src/postgresql/src/backend/access/transam/xloginsert.c:497:10: note: 'hole_length' was declared here >> uint16 hole_length; >> >> I've not checked whether they're spurious or not. Thanks for the report! We can suppress those warnings just by initializing cbimg.hole_length and hole_length. But I'd like to apply the attached patch which not only initializes the variable but also refactors the related code. Regards, -- Fujii Masao
Вложения
Re: pgsql: Add GUC to enable compression of full page images stored in WAL.
От
Michael Paquier
Дата:
On Thu, Mar 12, 2015 at 7:13 PM, Fujii Masao wrote: > On Wed, Mar 11, 2015 at 11:30 PM, Michael Paquier wrote: >> On Wed, Mar 11, 2015 at 11:18 PM, Andres Freund wrote: >>> I've not checked whether they're spurious or not. > > Thanks for the report! We can suppress those warnings just by > initializing cbimg.hole_length and hole_length. But I'd like to apply > the attached patch which not only initializes the variable but also > refactors the related code. FWIW, I just had a look at this patch, and it looks good to me. Thanks! -- Michael
Re: pgsql: Add GUC to enable compression of full page images stored in WAL.
От
Peter Eisentraut
Дата:
On 3/12/15 6:13 AM, Fujii Masao wrote: > On Wed, Mar 11, 2015 at 11:30 PM, Michael Paquier > <michael.paquier@gmail.com> wrote: >> On Wed, Mar 11, 2015 at 11:18 PM, Andres Freund <andres@2ndquadrant.com> wrote: >>> On 2015-03-11 06:54:16 +0000, Fujii Masao wrote: >>>> Add GUC to enable compression of full page images stored in WAL. >>> >>> This triggers a couple warnings here (gcc 4.9 and 5): >>> >>> /home/andres/src/postgresql/src/backend/access/transam/xloginsert.c: In function 'XLogInsert': >>> /home/andres/src/postgresql/src/backend/access/transam/xloginsert.c:670:5: warning: 'cbimg.hole_length' may be used uninitializedin this function [-Wmaybe-uninitialized] >>> memcpy(scratch, &cbimg, >>> ^ >>> /home/andres/src/postgresql/src/backend/access/transam/xloginsert.c:494:33: note: 'cbimg.hole_length' was declared here >>> XLogRecordBlockCompressHeader cbimg; >>> ^ >>> /home/andres/src/postgresql/src/backend/access/transam/xloginsert.c:668:20: warning: 'hole_length' may be used uninitializedin this function [-Wmaybe-uninitialized] >>> if (hole_length != 0 && is_compressed) >>> ^ >>> /home/andres/src/postgresql/src/backend/access/transam/xloginsert.c:497:10: note: 'hole_length' was declared here >>> uint16 hole_length; >>> >>> I've not checked whether they're spurious or not. > > Thanks for the report! We can suppress those warnings just by > initializing cbimg.hole_length and hole_length. But I'd like to apply > the attached patch which not only initializes the variable but also > refactors the related code. Could you please fix this compiler warning soon?
On Sun, Mar 15, 2015 at 5:23 AM, Peter Eisentraut <peter_e@gmx.net> wrote: > On 3/12/15 6:13 AM, Fujii Masao wrote: >> On Wed, Mar 11, 2015 at 11:30 PM, Michael Paquier >> <michael.paquier@gmail.com> wrote: >>> On Wed, Mar 11, 2015 at 11:18 PM, Andres Freund <andres@2ndquadrant.com> wrote: >>>> On 2015-03-11 06:54:16 +0000, Fujii Masao wrote: >>>>> Add GUC to enable compression of full page images stored in WAL. >>>> >>>> This triggers a couple warnings here (gcc 4.9 and 5): >>>> >>>> /home/andres/src/postgresql/src/backend/access/transam/xloginsert.c: In function 'XLogInsert': >>>> /home/andres/src/postgresql/src/backend/access/transam/xloginsert.c:670:5: warning: 'cbimg.hole_length' may be useduninitialized in this function [-Wmaybe-uninitialized] >>>> memcpy(scratch, &cbimg, >>>> ^ >>>> /home/andres/src/postgresql/src/backend/access/transam/xloginsert.c:494:33: note: 'cbimg.hole_length' was declared here >>>> XLogRecordBlockCompressHeader cbimg; >>>> ^ >>>> /home/andres/src/postgresql/src/backend/access/transam/xloginsert.c:668:20: warning: 'hole_length' may be used uninitializedin this function [-Wmaybe-uninitialized] >>>> if (hole_length != 0 && is_compressed) >>>> ^ >>>> /home/andres/src/postgresql/src/backend/access/transam/xloginsert.c:497:10: note: 'hole_length' was declared here >>>> uint16 hole_length; >>>> >>>> I've not checked whether they're spurious or not. >> >> Thanks for the report! We can suppress those warnings just by >> initializing cbimg.hole_length and hole_length. But I'd like to apply >> the attached patch which not only initializes the variable but also >> refactors the related code. > > Could you please fix this compiler warning soon? Sorry for the delay. Pushed. Regards, -- Fujii Masao