pgsql: Add GUC to enable compression of full page images stored in WAL.

Started by Fujii Masaoover 11 years ago7 messagescomitters
Jump to latest
#1Fujii Masao
masao.fujii@gmail.com

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(-)

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#2Andres Freund
andres@anarazel.de
In reply to: Fujii Masao (#1)
Re: pgsql: Add GUC to enable compression of full page images stored in WAL.

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 uninitialized 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 uninitialized in 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

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#3Michael Paquier
michael@paquier.xyz
In reply to: Andres Freund (#2)
Re: pgsql: Add GUC to enable compression of full page images stored in WAL.

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 uninitialized 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 uninitialized in 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

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#4Fujii Masao
masao.fujii@gmail.com
In reply to: Michael Paquier (#3)
Re: pgsql: Add GUC to enable compression of full page images stored in WAL.

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 uninitialized 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 uninitialized in 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

Attachments:

bugfix.patchtext/x-patch; charset=US-ASCII; name=bugfix.patchDownload+34-39
#5Michael Paquier
michael@paquier.xyz
In reply to: Fujii Masao (#4)
Re: pgsql: Add GUC to enable compression of full page images stored in WAL.

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

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#6Peter Eisentraut
peter_e@gmx.net
In reply to: Fujii Masao (#4)
Re: pgsql: Add GUC to enable compression of full page images stored in WAL.

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 uninitialized 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 uninitialized in 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?

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#7Fujii Masao
masao.fujii@gmail.com
In reply to: Peter Eisentraut (#6)
Re: pgsql: Add GUC to enable compression of full page images stored in WAL.

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 used uninitialized 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 uninitialized in 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

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers