Thanks for reviewing, Peter.

On 3/9/18 5:23 PM, Peter Eisentraut wrote:
> This seems like a useful test.
> 
> On 3/5/18 12:35, David Steele wrote:
>> +mkdir($tablespaceDir)
>> +    or die "unable to mkdir \"$tablespaceDir\"";
> 
> Use BAIL_OUT instead of die in tests.

Done.

>> +    $ts1UnloggedPath = $node->safe_psql('postgres',
>> +    q{select pg_relation_filepath('ts1_unlogged')});
> 
> strange indentation

Sure is.  Fixed.

>> +# Write forks to test that they are removed during recovery
>> +$node->command_ok(['touch', "$pgdata/${baseUnloggedPath}_vm"],
>> +    'touch vm fork in base');
>> +$node->command_ok(['touch', "$pgdata/${baseUnloggedPath}_fsm"],
>> +    'touch fsm fork in base');
> 
> These are not tests, just some prep work.  So they should not use
> command_ok.

Removed command_ok().

> It would probably also be better to avoid the Unix-specific touch
> command and instead use Perl code to open and write to the files.

Updated to use append_to_file().

>> +# Check unlogged table in base
>> +ok(-f "$pgdata/${baseUnloggedPath}_init", 'init fork in base');
>> +ok(-f "$pgdata/$baseUnloggedPath", 'main fork in base');
>> +ok(!-f "$pgdata/${baseUnloggedPath}_vm", 'vm fork not in base');
>> +ok(!-f "$pgdata/${baseUnloggedPath}_fsm", 'fsm fork not in base');
> 
> These test names could be a bit more verbose and distinct, for example,
> 'main fork was recreated after restart'.

Done.

A new patch is attached.

Thanks,
-- 
-David
da...@pgmasters.net
>From 6e082e9db8f401d986a03d02023173e37063996b Mon Sep 17 00:00:00 2001
From: David Steele <da...@pgmasters.net>
Date: Tue, 13 Mar 2018 10:06:09 -0400
Subject: [PATCH 1/1] Add regression tests for reinit.c.

These were originally written for a patch that refactored reinit.c. That 
refactor ended up not being needed, but it seems like a good idea to add the 
tests.
---
 src/test/recovery/t/014_unlogged_reinit.pl | 97 ++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)
 create mode 100644 src/test/recovery/t/014_unlogged_reinit.pl

diff --git a/src/test/recovery/t/014_unlogged_reinit.pl 
b/src/test/recovery/t/014_unlogged_reinit.pl
new file mode 100644
index 0000000000..e87c805e46
--- /dev/null
+++ b/src/test/recovery/t/014_unlogged_reinit.pl
@@ -0,0 +1,97 @@
+# Tests that unlogged tables are properly reinitialized after a crash.
+#
+# The behavior should be the same when restoring from a backup but that is not
+# tested here.
+use strict;
+use warnings;
+use PostgresNode;
+use TestLib;
+use Test::More tests => 12;
+
+# Initialize node without replication settings
+my $node = get_new_node('main');
+
+$node->init;
+$node->start;
+my $pgdata = $node->data_dir;
+
+# Create an unlogged table to test that forks other than init are not copied
+$node->safe_psql('postgres', 'CREATE UNLOGGED TABLE base_unlogged (id int)');
+
+my $baseUnloggedPath = $node->safe_psql('postgres',
+       q{select pg_relation_filepath('base_unlogged')});
+
+# Make sure main and init forks exist
+ok(-f "$pgdata/${baseUnloggedPath}_init", 'init fork in base');
+ok(-f "$pgdata/$baseUnloggedPath", 'main fork in base');
+
+# Create unlogged tables in a tablespace
+my $tablespaceDir = undef;
+my $ts1UnloggedPath = undef;
+
+$tablespaceDir = TestLib::tempdir . "/ts1";
+
+mkdir($tablespaceDir)
+       or BAIL_OUT("unable to mkdir '$tablespaceDir'");
+
+$node->safe_psql('postgres',
+       "CREATE TABLESPACE ts1 LOCATION '$tablespaceDir'");
+$node->safe_psql('postgres',
+       'CREATE UNLOGGED TABLE ts1_unlogged (id int) TABLESPACE ts1');
+
+$ts1UnloggedPath = $node->safe_psql('postgres',
+       q{select pg_relation_filepath('ts1_unlogged')});
+
+# Make sure main and init forks exist
+ok(-f "$pgdata/${ts1UnloggedPath}_init", 'init fork in tablespace');
+ok(-f "$pgdata/$ts1UnloggedPath", 'main fork in tablespace');
+
+# Crash the postmaster
+$node->stop('immediate');
+
+# Write forks to test that they are removed during recovery
+append_to_file("$pgdata/${baseUnloggedPath}_vm", 'TEST_VM');
+append_to_file("$pgdata/${baseUnloggedPath}_fsm", 'TEST_FSM');
+
+# Remove main fork to test that it is recopied from init
+unlink("$pgdata/${baseUnloggedPath}")
+       or BAIL_OUT("unable to remove '${baseUnloggedPath}'");
+
+# Write forks to test that they are removed by recovery
+append_to_file("$pgdata/${ts1UnloggedPath}_vm", 'TEST_VM');
+append_to_file("$pgdata/${ts1UnloggedPath}_fsm", 'TEST_FSM');
+
+# Remove main fork to test that it is recopied from init
+unlink("$pgdata/${ts1UnloggedPath}")
+       or BAIL_OUT("unable to remove '${ts1UnloggedPath}'");
+
+# Start the postmaster
+$node->start;
+
+# Check unlogged table in base
+ok(-f "$pgdata/${baseUnloggedPath}_init", 'init fork still exists in base');
+ok(-f "$pgdata/$baseUnloggedPath", 'main fork in base recreated at startup');
+ok(!-f "$pgdata/${baseUnloggedPath}_vm", 'vm fork in base removed at startup');
+ok(!-f "$pgdata/${baseUnloggedPath}_fsm",
+       'fsm fork in base removed at startup');
+
+# Drop unlogged table
+$node->safe_psql('postgres', 'DROP TABLE base_unlogged');
+
+# Check unlogged table in tablespace
+ok(-f "$pgdata/${ts1UnloggedPath}_init",
+       'init fork still exists in tablespace');
+ok(-f "$pgdata/$ts1UnloggedPath",
+       'main fork in tablspace recreated at startup');
+ok(!-f "$pgdata/${ts1UnloggedPath}_vm",
+       'vm fork in tablespace removed at startup');
+ok(!-f "$pgdata/${ts1UnloggedPath}_fsm",
+       'fsm fork in tablespace removed at startup');
+
+# Drop unlogged table
+$node->safe_psql('postgres', 'DROP TABLE ts1_unlogged');
+
+# Drop tablespace
+$node->safe_psql('postgres', 'DROP TABLESPACE ts1');
+rmdir($tablespaceDir)
+       or BAIL_OUT("unable to rmdir '$tablespaceDir'");
-- 
2.14.3 (Apple Git-98)

Reply via email to