Re: TAP test for recovery_end_command

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Amul Sul <sulamul(at)gmail(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Euler Taveira <euler(at)eulerto(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: TAP test for recovery_end_command
Date: 2021-10-27 04:07:46
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Mon, Oct 25, 2021 at 02:42:28PM +0530, Amul Sul wrote:
> Understood, moved tests to in the attached version.

Thanks for the new patch. I have reviewed its contents, and there
were a couple of things that caught my attention while putting my
hands on it.

+ "archive_cleanup_command = 'echo archive_cleanuped > $archive_cleanup_command_file'");
+ "recovery_end_command = 'echo recovery_ended > $recovery_end_command_file'");
This can be formatted with a single append_conf() call and qq() to
have the correct set of quotes.

+$node_primary->safe_psql('postgres', "CHECKPOINT");
my $current_lsn =
$node_primary->safe_psql('postgres', "SELECT pg_current_wal_lsn();");
This had better say that the checkpoint is necessary because we need
one before switching to a new segment on the primary, as much as the
checkpoint on the first standby is needed to trigger the command whose
execution is checked.

+ "archive_cleanup_command = 'echo xyz > $data_dir/unexisting_dir/xyz.file'");
+ "recovery_end_command = 'echo xyz > $data_dir/unexisting_dir/xyz.file'");
+# 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");

This SQL test is mostly useless IMO, as the promote() call done above
ensures that this state is reached properly, and the same thing could
be with the removals of RECOVERYHISTORY and RECOVERYXLOG. I think
that it would be better to check directly if the commands are run or
not. This is simple to test: look at the logs from a position just
before the promotion, slurp the log file of $standby2 from this
position, and finally compare its contents with a regex of your
choice. I have chosen a simple "qr/WARNING:.*recovery_end_command/s"
for the purpose of this test. Having a test for
archive_cleanup_command here would be nice, but that would be much
more costly than the end-of-recovery command, so I have left that
out. Perhaps we could just append it in the conf as a dummy, as you
did, though, but its execution is not deterministic in this test so we
are better without for now IMO.

perltidy was also complaining a bit, this is fixed as of the attached.
I have checked things on my own Windows dev box, while on it.

Attachment Content-Type Size
v6-0001-TAP-test-for-recovery_end_command-and-archive_cle.patch text/x-diff 4.2 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Bossart, Nathan 2021-10-27 04:10:07 Re: parallelizing the archiver
Previous Message Amit Kapila 2021-10-27 03:34:57 Re: Skipping logical replication transactions on subscriber side