On 03/16/18 17:14, Daniel Gustafsson wrote:
> The attached patch adds the test, and a neccessary extension to 
> check_pg_config
> to allow for extracting values from pg_config.h as opposed to just returning
> the number of regex matches. (needed for XLOG_BLCKSZ.)

Thanks for the review. I notice that cfbot has now flagged the patch as
failing, and when I look into it, it appears that cfbot is building with
your test patch, and without the xlog.c patch, and so the test naturally
fails. Does the cfbot require both patches to be attached to the same
email, in order to include them both? I'll try attaching both to this one,
and see what it does.

This is good confirmation that the test is effective. :)

-Chap
From 35684bdaa1d27c3f4b7198541f3c92bb2b4cb2f4 Mon Sep 17 00:00:00 2001
From: Chapman Flack <c...@anastigmatix.net>
Date: Sun, 25 Feb 2018 11:44:47 -0500
Subject: [PATCH] Zero headers of unused pages after WAL switch.

When writing zeroed pages to the remainder of a WAL segment
after a WAL switch, ensure that the headers of those pages are
also zeroed, as their initialized values otherwise reduce the
compressibility of the WAL segment file by general tools.
---
 src/backend/access/transam/xlog.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/backend/access/transam/xlog.c 
b/src/backend/access/transam/xlog.c
index 47a6c4d..a91ec7b 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -1556,7 +1556,16 @@ CopyXLogRecordToWAL(int write_len, bool isLogSwitch, 
XLogRecData *rdata,
                {
                        /* initialize the next page (if not initialized 
already) */
                        WALInsertLockUpdateInsertingAt(CurrPos);
-                       AdvanceXLInsertBuffer(CurrPos, false);
+                       /*
+                        * Fields in the page header preinitialized by 
AdvanceXLInsertBuffer
+                        * to nonconstant values reduce the compressibility of 
WAL segments,
+                        * and aren't needed in the freespace following a 
switch record.
+                        * Re-zero that header area. This is not 
performance-critical, as
+                        * the more empty pages there are for this loop to 
touch, the less
+                        * busy the system is.
+                        */
+                       currpos = GetXLogBuffer(CurrPos);
+                       MemSet(currpos, 0, SizeOfXLogShortPHD);
                        CurrPos += XLOG_BLCKSZ;
                }
        }
-- 
2.7.3

From a7cbcde7518e2f95673ec5ddba32b7ea6e0b84bb Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dan...@yesql.se>
Date: Fri, 16 Mar 2018 21:32:32 +0100
Subject: [PATCH] Add test for ensuring WAL segment is zeroed out

Switch to a new WAL file and ensure the last segment in the now
switched-away-from file is completely zeroed out.

This adds a mode to check_pg_config() where the match from the
regexp can be returned, not just the literal count of matches.
Only one match is returned (the first), but given the structure
of pg_config.h that's likely to be enough.
---
 src/test/perl/TestLib.pm             | 18 ++++++++++++++++--
 src/test/recovery/t/002_archiving.pl | 20 +++++++++++++++++++-
 2 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index 7d017d49bd..8b9796c4f6 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -227,15 +227,29 @@ sub append_to_file
 # that.
 sub check_pg_config
 {
-       my ($regexp) = @_;
+       my ($regexp, $value) = @_;
        my ($stdout, $stderr);
+       my $match;
        my $result = IPC::Run::run [ 'pg_config', '--includedir' ], '>',
          \$stdout, '2>', \$stderr
          or die "could not execute pg_config";
        chomp($stdout);
 
        open my $pg_config_h, '<', "$stdout/pg_config.h" or die "$!";
-       my $match = (grep {/^$regexp/} <$pg_config_h>);
+       if (defined $value)
+       {
+               while (<$pg_config_h>)
+               {
+                       $_ =~ m/^$regexp/;
+                       next unless defined $1;
+                       $match = $1;
+                       last;
+               }
+       }
+       else
+       {
+               $match = (grep {/^$regexp/} <$pg_config_h>);
+       }
        close $pg_config_h;
        return $match;
 }
diff --git a/src/test/recovery/t/002_archiving.pl 
b/src/test/recovery/t/002_archiving.pl
index e1bd3c95cc..c235e98904 100644
--- a/src/test/recovery/t/002_archiving.pl
+++ b/src/test/recovery/t/002_archiving.pl
@@ -3,7 +3,7 @@ use strict;
 use warnings;
 use PostgresNode;
 use TestLib;
-use Test::More tests => 1;
+use Test::More tests => 2;
 use File::Copy;
 
 # Initialize master node, doing archives
@@ -49,3 +49,21 @@ $node_standby->poll_query_until('postgres', $caughtup_query)
 my $result =
   $node_standby->safe_psql('postgres', "SELECT count(*) FROM tab_int");
 is($result, qq(1000), 'check content from archives');
+
+# Add some content, switch WAL file and read the last segment of the WAL
+# file which should be zeroed out
+my $current_file = $node_master->safe_psql('postgres',
+       "SELECT pg_walfile_name(pg_current_wal_lsn())");
+$node_master->safe_psql('postgres', "INSERT INTO tab_int VALUES (1)");
+$node_master->safe_psql('postgres', "SELECT pg_switch_wal()");
+
+my $xlog_blcksz = check_pg_config('#define XLOG_BLCKSZ (\d+)', 1);
+
+open my $fh, '<:raw', $node_master->data_dir . '/pg_wal/' . $current_file;
+# SEEK_END is defined as 2, but is not allowed when using strict mode
+seek $fh, ($xlog_blcksz * -1), 2;
+my $len = read($fh, my $bytes, $xlog_blcksz);
+close $fh;
+
+my ($wal_segment) = unpack 'N' . $xlog_blcksz, $bytes;
+is($wal_segment, 0, 'make sure wal segment is zeroed');
-- 
2.14.1.145.gb3622a4ee

Reply via email to