BRIN: mask BRIN_EVACUATE_PAGE for WAL consistency checking

That bit is unlogged and therefore it's wrong to consider it in WAL page
comparison.

Add a test that tickles the case, as branch testing technology allows.

This has been a problem ever since wal consistency checking was
introduced (commit a507b86900f6 for pg10), so backpatch to all supported
branches.

Author: 王海洋 (Haiyang Wang) <wanghaiyang.001@bytedance.com>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/CACciXAD2UvLMOhc4jX9VvOKt7DtYLr3OYRBhvOZ-jRxtzc_7Jg@mail.gmail.com
Discussion: https://postgr.es/m/CACciXADOfErX9Bx0nzE_SkdfXr6Bbpo5R=v_B6MUTEYW4ya+cg@mail.gmail.com

diff --git a/src/backend/access/brin/brin_pageops.c b/src/backend/access/brin/brin_pageops.c
index 17257919dbfac988ac3f554555f093c002aa1ed4..f17aad51b631958bdc19ee96febbe5edc7931e9a 100644 (file)
--- a/src/backend/access/brin/brin_pageops.c
+++ b/src/backend/access/brin/brin_pageops.c
@@ -541,7 +541,12 @@ brin_start_evacuating_page(Relation idxRel, Buffer buf)
        lp = PageGetItemId(page, off);
        if (ItemIdIsUsed(lp))
        {
-           /* prevent other backends from adding more stuff to this page */
+           /*
+            * Prevent other backends from adding more stuff to this page:
+            * BRIN_EVACUATE_PAGE informs br_page_get_freespace that this page
+            * can no longer be used to add new tuples.  Note that this flag
+            * is not WAL-logged, except accidentally.
+            */
            BrinPageFlags(page) |= BRIN_EVACUATE_PAGE;
            MarkBufferDirtyHint(buf, true);
 

diff --git a/src/backend/access/brin/brin_xlog.c b/src/backend/access/brin/brin_xlog.c
index d6fcea24d2d353ca2b53cfee54483523666b59a4..af6949882a15fca4b9b57f0da72a67d36483824c 100644 (file)
--- a/src/backend/access/brin/brin_xlog.c
+++ b/src/backend/access/brin/brin_xlog.c
@@ -358,4 +358,10 @@ brin_mask(char *pagedata, BlockNumber blkno)
    {
        mask_unused_space(page);
    }
+
+   /*
+    * BRIN_EVACUATE_PAGE is not WAL-logged, since it's of no use in recovery.
+    * Mask it.  See brin_start_evacuating_page() for details.
+    */
+   BrinPageFlags(page) &= ~BRIN_EVACUATE_PAGE;
 }

diff --git a/src/test/modules/brin/Makefile b/src/test/modules/brin/Makefile
index c8715939060ec2d70e41988b2392f68f7d71df1f..e74af891ce7f6d1c6fd8594f143bc421e2655488 100644 (file)
--- a/src/test/modules/brin/Makefile
+++ b/src/test/modules/brin/Makefile
@@ -1,6 +1,6 @@
 # src/test/modules/brin/Makefile
 
-EXTRA_INSTALL = contrib/pageinspect
+EXTRA_INSTALL = contrib/pageinspect contrib/pg_walinspect
 
 ISOLATION = summarization-and-inprogress-insertion
 TAP_TESTS = 1

diff --git a/src/test/modules/brin/t/02_wal_consistency.pl b/src/test/modules/brin/t/02_wal_consistency.pl
new file mode 100644 (file)
index 0000000..cc465bc
--- /dev/null
+++ b/src/test/modules/brin/t/02_wal_consistency.pl
@@ -0,0 +1,75 @@
+# Copyright (c) 2021-2022, PostgreSQL Global Development Group
+
+# Verify WAL consistency
+
+use strict;
+use warnings;
+
+use PostgreSQL::Test::Utils;
+use Test::More;
+use PostgreSQL::Test::Cluster;
+
+# Set up primary
+my $whiskey = PostgreSQL::Test::Cluster->new('whiskey');
+$whiskey->init(allows_streaming => 1);
+$whiskey->append_conf('postgresql.conf', 'wal_consistency_checking = brin');
+$whiskey->start;
+$whiskey->safe_psql('postgres', 'create extension pageinspect');
+$whiskey->safe_psql('postgres', 'create extension pg_walinspect');
+is( $whiskey->psql(
+       'postgres',
+       qq[SELECT pg_create_physical_replication_slot('standby_1');]),
+   0,
+   'physical slot created on primary');
+
+# Take backup
+my $backup_name = 'brinbkp';
+$whiskey->backup($backup_name);
+
+# Create streaming standby linking to primary
+my $charlie = PostgreSQL::Test::Cluster->new('charlie');
+$charlie->init_from_backup($whiskey, $backup_name, has_streaming => 1);
+$charlie->append_conf('postgresql.conf', 'primary_slot_name = standby_1');
+$charlie->start;
+
+# Now write some WAL in the primary
+
+$whiskey->safe_psql(
+   'postgres', qq{
+create table tbl_timestamp0 (d1 timestamp(0) without time zone) with (fillfactor=10);
+create index on tbl_timestamp0 using brin (d1) with (pages_per_range = 1, autosummarize=false);
+});
+my $start_lsn = $whiskey->lsn('insert');
+# Run a loop that will end when the second revmap page is created
+$whiskey->safe_psql(
+   'postgres', q{
+do
+$$
+declare
+  current timestamp with time zone := '2019-03-27 08:14:01.123456789 America/Punta_Arenas';
+begin
+  loop
+    insert into tbl_timestamp0 select i from
+      generate_series(current, current + interval '1 day', '28 seconds') i;
+    perform brin_summarize_new_values('tbl_timestamp0_d1_idx');
+    if (brin_metapage_info(get_raw_page('tbl_timestamp0_d1_idx', 0))).lastrevmappage > 1 then
+      exit;
+    end if;
+    current := current + interval '1 day';
+  end loop;
+end
+$$;
+});
+my $end_lsn = $whiskey->lsn('insert');
+
+my ($ret, $out, $err) = $whiskey->psql(
+   'postgres', qq{
+   select count(*) from pg_get_wal_records_info('$start_lsn', '$end_lsn')
+   where resource_manager = 'BRIN' AND
+   record_type ILIKE '%revmap%'
+   });
+cmp_ok($out, '>=', 1);
+
+$whiskey->wait_for_catchup($charlie, 'replay', $whiskey->lsn('insert'));
+
+done_testing();