public inbox for [email protected]  
help / color / mirror / Atom feed
From: Noah Misch <[email protected]>
To: Michael Paquier <[email protected]>
Cc: Nathan Bossart <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Robert Pang <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: [email protected]
Cc: [email protected]
Subject: Re: Back-patch of: avoid multiple hard links to same WAL file after a crash
Date: Wed, 2 Apr 2025 17:29:00 -0700
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CAJhEC04tBkYPF4q2uS_rCytauvNEVqdBAzasBEokfceFhF=KDQ@mail.gmail.com>
	<Z2L6e1w-xABVTBRR@nathan>
	<7chjz7zeigbsbt7nim4cj6zryflzcpy2lgenas3yh7cpvwf3gb@m6cwqzlvfd6q>
	<[email protected]>
	<[email protected]>
	<Z8n7HopB0bb4_j19@nathan>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>

On Tue, Mar 11, 2025 at 06:23:15PM -0700, Noah Misch wrote:
> On Wed, Mar 12, 2025 at 09:46:27AM +0900, Michael Paquier wrote:
> > On Tue, Mar 11, 2025 at 01:57:49PM -0700, Noah Misch wrote:
> > > Thanks for crafting back-branch versions.  I've queued a task to confirm I get
> > > the same result.
> > 
> > Thanks for that.  That helps a lot.
> 
> I'll let you know when I get there.

Your back-patches are correct.  Thanks.

> > > There's a test case I'll polish, too.
> > 
> > Are you considering the addition of a TAP test in 17~ based on a wait
> > injection point in the checkpointer coupled with a check of the server
> > logs to see if we see the error patterns you've spotted?
> 
> No, nothing involving injection points or otherwise version-specific.

Here it is.  Making it fail three times took looping 1383s, 5841s, and 2594s.
Hence, it couldn't be expected to catch the regression before commit, but it
would have made sufficient buildfarm and CI noise in the day after commit.

Author:     Noah Misch <[email protected]>
Commit:     Noah Misch <[email protected]>

    Test restartpoints in archive recovery.
    
    v14 commit 1f95181b44c843729caaa688f74babe9403b5850 and its v13 equivalent
    caused timing-dependent failures in archive recovery, at restartpoints.  The
    symptom was "invalid magic number 0000 in log segment X, offset 0",
    "unexpected pageaddr X in log segment Y, offset 0" [X < Y], or an assertion
    failure.  Commit FIXME and predecessors back-patched v15 changes to fix that.
    This test reproduces the problem probabilistically, typically in less than
    1000 iterations of the test.  Hence, buildfarm and CI runs would have surfaced
    enough failures to get attention within a day.  Back-patch to v13 (all
    supported versions).
    
    Reported by Arun Thirupathi.  Reviewed by FIXME.
    
    Discussion: https://postgr.es/m/[email protected]

diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build
index 057bcde..cb98376 100644
--- a/src/test/recovery/meson.build
+++ b/src/test/recovery/meson.build
@@ -53,6 +53,7 @@ tests += {
       't/042_low_level_backup.pl',
       't/043_no_contrecord_switch.pl',
       't/044_invalidate_inactive_slots.pl',
+      't/045_archive_restartpoint.pl',
     ],
   },
 }
diff --git a/src/test/recovery/t/045_archive_restartpoint.pl b/src/test/recovery/t/045_archive_restartpoint.pl
new file mode 100644
index 0000000..b143bc4
--- /dev/null
+++ b/src/test/recovery/t/045_archive_restartpoint.pl
@@ -0,0 +1,57 @@
+
+# Copyright (c) 2024-2025, PostgreSQL Global Development Group
+
+# Test restartpoints during archive recovery.
+use strict;
+use warnings;
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $archive_max_mb = 320;
+my $wal_segsize = 1;
+
+# Initialize primary node
+my $node_primary = PostgreSQL::Test::Cluster->new('primary');
+$node_primary->init(
+	has_archiving => 1,
+	allows_streaming => 1,
+	extra => [ '--wal-segsize' => $wal_segsize ]);
+$node_primary->start;
+my $backup_name = 'my_backup';
+$node_primary->backup($backup_name);
+
+$node_primary->safe_psql('postgres',
+	('DO $$BEGIN FOR i IN 1..' . $archive_max_mb / $wal_segsize)
+	  . ' LOOP CHECKPOINT; PERFORM pg_switch_wal(); END LOOP; END$$;');
+
+# Force archiving of WAL file containing recovery target
+my $until_lsn = $node_primary->lsn('write');
+$node_primary->safe_psql('postgres', "SELECT pg_switch_wal()");
+$node_primary->stop;
+
+# Archive recovery
+my $node_restore = PostgreSQL::Test::Cluster->new('restore');
+$node_restore->init_from_backup($node_primary, $backup_name,
+	has_restoring => 1);
+$node_restore->append_conf('postgresql.conf',
+	"recovery_target_lsn = '$until_lsn'");
+$node_restore->append_conf('postgresql.conf',
+	'recovery_target_action = pause');
+$node_restore->append_conf('postgresql.conf',
+	'max_wal_size = ' . 2 * $wal_segsize);
+$node_restore->append_conf('postgresql.conf', 'log_checkpoints = on');
+
+$node_restore->start;
+
+# Wait until restore has replayed enough data
+my $caughtup_query =
+  "SELECT '$until_lsn'::pg_lsn <= pg_last_wal_replay_lsn()";
+$node_restore->poll_query_until('postgres', $caughtup_query)
+  or die "Timed out while waiting for restore to catch up";
+
+$node_restore->stop;
+ok(1, 'restore caught up');
+
+done_testing();


Attachments:

  [text/plain] test-archive-restartpoint-v1.patch (3.3K, ../[email protected]/2-test-archive-restartpoint-v1.patch)
  download | inline diff:
Author:     Noah Misch <[email protected]>
Commit:     Noah Misch <[email protected]>

    Test restartpoints in archive recovery.
    
    v14 commit 1f95181b44c843729caaa688f74babe9403b5850 and its v13 equivalent
    caused timing-dependent failures in archive recovery, at restartpoints.  The
    symptom was "invalid magic number 0000 in log segment X, offset 0",
    "unexpected pageaddr X in log segment Y, offset 0" [X < Y], or an assertion
    failure.  Commit FIXME and predecessors back-patched v15 changes to fix that.
    This test reproduces the problem probabilistically, typically in less than
    1000 iterations of the test.  Hence, buildfarm and CI runs would have surfaced
    enough failures to get attention within a day.  Back-patch to v13 (all
    supported versions).
    
    Reported by Arun Thirupathi.  Reviewed by FIXME.
    
    Discussion: https://postgr.es/m/[email protected]

diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build
index 057bcde..cb98376 100644
--- a/src/test/recovery/meson.build
+++ b/src/test/recovery/meson.build
@@ -53,6 +53,7 @@ tests += {
       't/042_low_level_backup.pl',
       't/043_no_contrecord_switch.pl',
       't/044_invalidate_inactive_slots.pl',
+      't/045_archive_restartpoint.pl',
     ],
   },
 }
diff --git a/src/test/recovery/t/045_archive_restartpoint.pl b/src/test/recovery/t/045_archive_restartpoint.pl
new file mode 100644
index 0000000..b143bc4
--- /dev/null
+++ b/src/test/recovery/t/045_archive_restartpoint.pl
@@ -0,0 +1,57 @@
+
+# Copyright (c) 2024-2025, PostgreSQL Global Development Group
+
+# Test restartpoints during archive recovery.
+use strict;
+use warnings;
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $archive_max_mb = 320;
+my $wal_segsize = 1;
+
+# Initialize primary node
+my $node_primary = PostgreSQL::Test::Cluster->new('primary');
+$node_primary->init(
+	has_archiving => 1,
+	allows_streaming => 1,
+	extra => [ '--wal-segsize' => $wal_segsize ]);
+$node_primary->start;
+my $backup_name = 'my_backup';
+$node_primary->backup($backup_name);
+
+$node_primary->safe_psql('postgres',
+	('DO $$BEGIN FOR i IN 1..' . $archive_max_mb / $wal_segsize)
+	  . ' LOOP CHECKPOINT; PERFORM pg_switch_wal(); END LOOP; END$$;');
+
+# Force archiving of WAL file containing recovery target
+my $until_lsn = $node_primary->lsn('write');
+$node_primary->safe_psql('postgres', "SELECT pg_switch_wal()");
+$node_primary->stop;
+
+# Archive recovery
+my $node_restore = PostgreSQL::Test::Cluster->new('restore');
+$node_restore->init_from_backup($node_primary, $backup_name,
+	has_restoring => 1);
+$node_restore->append_conf('postgresql.conf',
+	"recovery_target_lsn = '$until_lsn'");
+$node_restore->append_conf('postgresql.conf',
+	'recovery_target_action = pause');
+$node_restore->append_conf('postgresql.conf',
+	'max_wal_size = ' . 2 * $wal_segsize);
+$node_restore->append_conf('postgresql.conf', 'log_checkpoints = on');
+
+$node_restore->start;
+
+# Wait until restore has replayed enough data
+my $caughtup_query =
+  "SELECT '$until_lsn'::pg_lsn <= pg_last_wal_replay_lsn()";
+$node_restore->poll_query_until('postgres', $caughtup_query)
+  or die "Timed out while waiting for restore to catch up";
+
+$node_restore->stop;
+ok(1, 'restore caught up');
+
+done_testing();


view thread (15+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Back-patch of: avoid multiple hard links to same WAL file after a crash
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox