On Wed, Oct 20, 2021 at 11:09 AM Michael Paquier <[email protected]> wrote:
>
> On Wed, Oct 06, 2021 at 06:49:10PM +0530, Amul Sul wrote:
> > Thanks for the suggestion, added the same in the attached version.
>
> Hmm. The run-time of 020_archive_status.p bumps from 4.7s to 5.8s on
> my laptop, so the change is noticeable. I agree that it would be good
> to have more coverage for those commands, but I also think that we
> should make things cheaper if we can, particularly knowing that those
> commands just touch a file. This patch creates two stanbys for its
> purpose, but do we need that much?
>
> On top of that, 020_archive_status.pl does not look like the correct
> place for this set of tests. 002_archiving.pl would be a better
> candidate, where we already have two standbys that get promoted, so
> you could have both the failure and success cases there. There should
> be no need for extra wait phases either.
>
Understood, moved tests to 002_archiving.pl in the attached version.
> +$standby4->append_conf('postgresql.conf',
> + "recovery_end_command = 'echo recovery_ended > /non_existing_dir/file'");
> I am wondering how this finishes on Windows.
My colleague Neha Sharma has confirmed that the attached version is
passing on the Windows.
Regards,
Amul
From a340aaaf9298e6adfd322afb82f825f296650375 Mon Sep 17 00:00:00 2001
From: Amul Sul <[email protected]>
Date: Mon, 25 Oct 2021 03:38:25 -0400
Subject: [PATCH v5] TAP test for recovery_end_command and
archive_cleanup_command
---
src/test/recovery/t/002_archiving.pl | 48 +++++++++++++++++++++++-----
1 file changed, 40 insertions(+), 8 deletions(-)
diff --git a/src/test/recovery/t/002_archiving.pl b/src/test/recovery/t/002_archiving.pl
index 24852c97fda..de7b10c8f85 100644
--- a/src/test/recovery/t/002_archiving.pl
+++ b/src/test/recovery/t/002_archiving.pl
@@ -6,7 +6,7 @@ use strict;
use warnings;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 3;
+use Test::More tests => 6;
use File::Copy;
# Initialize primary node, doing archives
@@ -28,11 +28,22 @@ $node_standby->init_from_backup($node_primary, $backup_name,
has_restoring => 1);
$node_standby->append_conf('postgresql.conf',
"wal_retrieve_retry_interval = '100ms'");
+
+# Set archive_cleanup_command and recovery_end_command
+my $data_dir = $node_standby->data_dir;
+my $archive_cleanup_command_file = "$data_dir/archive_cleanup_command.done";
+my $recovery_end_command_file = "$data_dir/recovery_end_command.done";
+$node_standby->append_conf('postgresql.conf',
+ "archive_cleanup_command = 'echo archive_cleanuped > $archive_cleanup_command_file'");
+$node_standby->append_conf('postgresql.conf',
+ "recovery_end_command = 'echo recovery_ended > $recovery_end_command_file'");
+
$node_standby->start;
# Create some content on primary
$node_primary->safe_psql('postgres',
"CREATE TABLE tab_int AS SELECT generate_series(1,1000) AS a");
+$node_primary->safe_psql('postgres', "CHECKPOINT");
my $current_lsn =
$node_primary->safe_psql('postgres', "SELECT pg_current_wal_lsn();");
@@ -53,18 +64,34 @@ my $result =
$node_standby->safe_psql('postgres', "SELECT count(*) FROM tab_int");
is($result, qq(1000), 'check content from archives');
-# Check the presence of temporary files specifically generated during
-# archive recovery. To ensure the presence of the temporary history
-# file, switch to a timeline large enough to allow a standby to recover
-# a history file from an archive. As this requires at least two timeline
-# switches, promote the existing standby first. Then create a second
-# standby based on the promoted one. Finally, the second standby is
-# promoted.
+# archive_cleanup_command executed with every restart point and that can be
+# trigger using checkpoint
+$node_standby->safe_psql('postgres', q{CHECKPOINT});
+ok(-f "$archive_cleanup_command_file",
+ 'archive_cleanup_command executed on checkpoint');
+
+# Check recovery_end_command executed on recovery end which on promotion and the
+# presence of temporary files specifically generated during archive recovery.
+# To ensure the presence of the temporary history file, switch to a timeline
+# large enough to allow a standby to recover a history file from an archive. As
+# this requires at least two timeline switches, promote the existing standby
+# first. Then create a second standby based on the promoted one. Finally, the
+# second standby is promoted.
$node_standby->promote;
+ok(-f "$recovery_end_command_file",
+ 'recovery_end_command executed after promotion');
my $node_standby2 = PostgreSQL::Test::Cluster->new('standby2');
$node_standby2->init_from_backup($node_primary, $backup_name,
has_restoring => 1);
+
+# Check effect of failing archive_cleanup_command and recovery_end_command
+# execution on the promotion by setting wrong command.
+$node_standby2->append_conf('postgresql.conf',
+ "archive_cleanup_command = 'echo xyz > $data_dir/unexisting_dir/xyz.file'");
+$node_standby2->append_conf('postgresql.conf',
+ "recovery_end_command = 'echo xyz > $data_dir/unexisting_dir/xyz.file'");
+
$node_standby2->start;
# Now promote standby2, and check that temporary files specifically
@@ -75,3 +102,8 @@ ok( !-f "$node_standby2_data/pg_wal/RECOVERYHISTORY",
"RECOVERYHISTORY removed after promotion");
ok( !-f "$node_standby2_data/pg_wal/RECOVERYXLOG",
"RECOVERYXLOG removed after promotion");
+
+# Failing to execute archive_cleanup_command and/or recovery_end_command does
+# not affect promotion.
+is($node_standby2->safe_psql( 'postgres', q{SELECT pg_is_in_recovery()}), 'f',
+ "standby promoted successfully despite incorrect archive_cleanup_command and recovery_end_command");
--
2.18.0
