Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 08af921

Browse files
committed
Fix order of shutdown cleanup operations in PostgresNode.pm.
Previously, database clusters created by a TAP test were shut down by DESTROY methods attached to the PostgresNode objects representing them. The trouble with that is that if the objects survive into the final global destruction phase (which they do), Perl executes the DESTROY methods in an unspecified order. Thus, the order of shutdown of multiple clusters was indeterminate, which might lead to not-very-reproducible errors getting logged (eg from a slave whose master might or might not get killed first). Worse, the File::Temp objects representing the temporary PGDATA directories might get destroyed before the PostgresNode objects, resulting in attempts to delete PGDATA directories that still have live servers in them. On Windows, this would lead to directory deletion failures; on Unix, it usually had no effects worse than erratic "could not open temporary statistics file "pg_stat/global.tmp": No such file or directory" log messages. While none of this would affect the reported result of the TAP test, which is already determined, it could be very confusing when one is trying to understand from the logs what went wrong with a failed test. To fix, do the postmaster shutdowns in an END block rather than at object destruction time. The END block will execute at a well-defined (and reasonable) time during script termination, and it will stop the postmasters in order of PostgresNode object creation. (Perhaps we should change that to be reverse order of creation, but the main point here is that we now have control which we did not before.) Use "pg_ctl stop", not an asynchronous kill(SIGQUIT), so that we wait for the postmasters to shut down before proceeding with directory deletion. Deletion of temporary directories still happens in an unspecified order during global destruction, but I can see no reason to care about that once the postmasters are stopped.
1 parent 82311bc commit 08af921

File tree

1 file changed

+17
-9
lines changed

1 file changed

+17
-9
lines changed

src/test/perl/PostgresNode.pm

+17-9
Original file line numberDiff line numberDiff line change
@@ -662,6 +662,7 @@ sub stop
662662
my $pgdata = $self->data_dir;
663663
my $name = $self->name;
664664
$mode = 'fast' unless defined $mode;
665+
return unless defined $self->{_pid};
665666
print "### Stopping node \"$name\" using mode $mode\n";
666667
TestLib::system_log('pg_ctl', '-D', $pgdata, '-m', $mode, 'stop');
667668
$self->{_pid} = undef;
@@ -826,8 +827,8 @@ sub _update_pid
826827
Build a new PostgresNode object, assigning a free port number. Standalone
827828
function that's automatically imported.
828829
829-
We also register the node, to avoid the port number from being reused
830-
for another node even when this one is not active.
830+
Remembers the node, to prevent its port number from being reused for another
831+
node, and to ensure that it gets shut down when the test script exits.
831832
832833
You should generally use this instead of PostgresNode::new(...).
833834
@@ -889,14 +890,21 @@ sub get_new_node
889890
return $node;
890891
}
891892

892-
# Attempt automatic cleanup
893-
sub DESTROY
893+
# Automatically shut down any still-running nodes when the test script exits.
894+
# Note that this just stops the postmasters (in the same order the nodes were
895+
# created in). Temporary PGDATA directories are deleted, in an unspecified
896+
# order, later when the File::Temp objects are destroyed.
897+
END
894898
{
895-
my $self = shift;
896-
my $name = $self->name;
897-
return unless defined $self->{_pid};
898-
print "### Signalling QUIT to $self->{_pid} for node \"$name\"\n";
899-
TestLib::system_log('pg_ctl', 'kill', 'QUIT', $self->{_pid});
899+
# take care not to change the script's exit value
900+
my $exit_code = $?;
901+
902+
foreach my $node (@all_nodes)
903+
{
904+
$node->teardown_node;
905+
}
906+
907+
$? = $exit_code;
900908
}
901909

902910
=pod

0 commit comments

Comments
 (0)