Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move PID deletion out of DEMOLISH method #79

Closed
wants to merge 1 commit into from
Closed

Move PID deletion out of DEMOLISH method #79

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Changes
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ Revision history for Perl extension PGXN::Manager
- Fixed invalid license example in the META spec.
- Added a logger to the Consumer and the Mastodon and Twitter handlers,
so that they now log debug and info messages about what's being sent.
- Moved PID file cleanup from the `DEMOLISH` method to the `run` method,
where it should always execute. This will hopefully fix the issue
where the consumer mysteriously ceases running and doesn't remove its
PID file, so never restarts.

0.31.1 2023-10-07T21:40:53Z
- Restored the writing of the pgxn_consumer PID file, accidentally
Expand Down
12 changes: 5 additions & 7 deletions lib/PGXN/Manager/Consumer.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,6 @@ sub _signal_handler {
};
}

sub DEMOLISH {
my $self = shift;
if (my $path = $self->pid_file) {
unlink $path;
}
}

sub run {
my $self = shift;
$self->log(INFO => sprintf "Starting %s %s", __PACKAGE__, __PACKAGE__->VERSION);
Expand All @@ -114,6 +107,11 @@ sub run {
sleep($self->interval);
}

if (my $path = $self->pid_file) {
$self->log(DEBUG => "Unlinking PID file ", $self->pid_file);
unlink $path;
}

$self->log(INFO => 'Shutting down');
return 0;
}
Expand Down
40 changes: 23 additions & 17 deletions t/consumer.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,19 +109,6 @@ LOGFILE: {

my $chans = join(', ', PGXN::Manager::Consumer::CHANNELS);

##############################################################################
# Test pid file cleanup.
PID: {
my $tmp = File::Temp->new(UNLINK => 0);
my $fn = $tmp->filename;
do {
my $consumer = $CLASS->new(logger => $logger, pid_file => $fn);
file_exists_ok $fn, 'PID file should exist';
};
file_not_exists_ok $fn, 'PID file be gone';
is output(), '', 'Should have no output';
}

##############################################################################
# Load the test environment configuration.
sub load_config {
Expand Down Expand Up @@ -327,22 +314,41 @@ RUN: {
$db_mocker->mock(do => sub { shift; push @done => \@_ });
my $exp_done = [map { ["LISTEN pgxn_$_"] } PGXN::Manager::Consumer::CHANNELS ];

# Run it.
# Set up a PID file.
my $tmp = File::Temp->new(UNLINK => 0);
my $fn = $tmp->filename;

# Set up the config.
local @ARGV = qw(--env test --interval 0);
my $cfg = $CLASS->_config;
$cfg->{logger} = $logger;
$cfg->{pid_file} = $fn;

# Instantiate.
my $consumer = $CLASS->new($cfg);
is $consumer->interval, 0, 'Should have interval 0';
is $consumer->continue, 1, 'Should have default continue 1';
is $consumer->verbose, 0, 'Should have default verbose 0';
is $consumer->logger, $logger, 'Should have set logger';
is $consumer->pid_file, $fn, 'Should have set pid_file';
file_exists_ok $fn, 'PID file should exist';

# Run it.
$logger->{verbose} = 2;
is $consumer->run, 0, 'Run consumer';
file_not_exists_ok $fn, 'PID file should no longer exist';
$logger->{verbose} = 1;

is_deeply output(), join("\n",
"$logtime - INFO: Starting $CLASS " . $CLASS->VERSION,
"$logtime - DEBUG: Loading PGXN::Manager::Consumer::mastodon",
"$logtime - DEBUG: Configuring PGXN::Manager::Consumer::mastodon for release",
"$logtime - DEBUG: Loading PGXN::Manager::Consumer::twitter",
"$logtime - DEBUG: Configuring PGXN::Manager::Consumer::twitter for release",
"$logtime - DEBUG: Unlinking PID file $fn",
"$logtime - INFO: Shutting down",
'',
), 'Should have startup and shutdown log entries';
), 'Should have startup, loading, PID, and shutdown log entries';
is_deeply \@done, $exp_done, 'Should have listened to all channels';
ok $consumer->conn->dbh->{Callbacks}{connected},
'Should have configured listening in connected callback';
Expand Down Expand Up @@ -412,7 +418,7 @@ CONSUME: {
# Set up a notification.
my $json1 = '{"name": "Julie ❤️"}';
my $payload1 = {name => 'Julie ❤️'};
my $consumer = $new_consumer->(verbose => 2, logger => $logger);
my $consumer = $new_consumer->(verbose => 2);
$consumer->conn->run(sub {
$_->do("SELECT pg_notify(?, ?)", undef, 'pgxn_release', $json1);
});
Expand Down Expand Up @@ -456,7 +462,7 @@ CONSUME: {
my $json3 = '{"drop": "out"}';
my $payload3 = {drop => 'out'};
$logger->{verbose} = 0;
$consumer = $new_consumer->(verbose => 0, logger => $logger);
$consumer = $new_consumer->(verbose => 0);
$consumer->conn->run(sub {
$_->do("SELECT pg_notify(?, ?)", undef, 'pgxn_drop', $json3);
});
Expand Down