From 8d9579fbd62b993074b8dfed8f889da48907d664 Mon Sep 17 00:00:00 2001 From: "David E. Wheeler" Date: Sat, 7 Oct 2023 12:13:43 -0400 Subject: [PATCH] Delete PID file on exit I had thought that Proc::Daemon did it, but apparently not, because the service was failing to restart when the host was reboot. This should fix it. Also test on Postgres 16. --- .github/workflows/ci.yml | 4 ++-- Changes | 1 + lib/PGXN/Manager/Consumer.pm | 13 +++++++++++-- t/consumer.t | 31 +++++++++++++++++++++++++------ 4 files changed, 39 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2d17065..a56084a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,7 +9,7 @@ jobs: build: strategy: matrix: - pg: [15, 14, 13, 12] + pg: [16, 15, 14, 13, 12] name: 🐘 PostgreSQL ${{ matrix.pg }} runs-on: ubuntu-latest container: pgxn/pgxn-tools @@ -19,7 +19,7 @@ jobs: - name: Start PostgreSQL ${{ matrix.pg }} run: |- pg-start ${{ matrix.pg }} postgresql-plperl-${{ matrix.pg }} libxml-parser-perl libarchive-zip-perl libarchive-extract-perl libaliased-perl libsoftware-license-perl libtap-parser-sourcehandler-pgtap-perl libtest-file-perl libtest-file-contents-perl libtest-harness-perl libtest-mockmodule-perl libtest-nowarnings-perl libtest-xml-perl libtest-xpath-perl libclass-isa-perl libdata-dump-perl libdata-validate-uri-perl libdbi-perl libdbix-connector-perl libemail-valid-perl libemail-mime-perl libemail-address-perl libencode-perl libexception-class-dbi-perl libhttp-body-perl libhttp-negotiate-perl libjson-xs-perl libmoose-perl libmoosex-singleton-perl libnamespace-autoclean-perl libplack-perl libplack-middleware-session-perl libplack-middleware-methodoverride-perl libsemver-perl libtemplate-declare-perl libtry-tiny-perl liburi-template-perl libplack-middleware-debug-perl libplack-middleware-reverseproxy-perl libtest-pod-perl libtest-pod-coverage-perl libtest-spelling-perl cpanminus - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Build PGXN Manager run: |- perl Build.PL diff --git a/Changes b/Changes index 9b3b75c..7e60687 100644 --- a/Changes +++ b/Changes @@ -3,6 +3,7 @@ Revision history for Perl extension PGXN::Manager 0.30.2 - Added the application names (pgxn_manager and pgxn_consumer) to the Postgres connections. + - Updated pgxn_consumer to delete the PID file on exit. 0.30.1 2023-02-18T23:15:06Z - Added the --log-file option to `pgxn_consumer`. diff --git a/lib/PGXN/Manager/Consumer.pm b/lib/PGXN/Manager/Consumer.pm index 3350c62..8636e26 100644 --- a/lib/PGXN/Manager/Consumer.pm +++ b/lib/PGXN/Manager/Consumer.pm @@ -24,6 +24,7 @@ use constant CHANNELS => qw(release new_user new_mirror); has verbose => (is => 'ro', isa => 'Int', required => 1, default => 0); has interval => (is => 'ro', isa => 'Num', required => 1, default => 5); has continue => (is => 'rw', isa => 'Bool', required => 1, default => 1); +has pid_file => (is => 'rw', isa => 'Str'); has log_fh => (is => 'ro', isa => 'IO::Handle', required => 1, default => sub { _log_fh() }); @@ -64,7 +65,7 @@ sub go { my $daemon = Proc::Daemon->new( work_dir => getcwd, dont_close_fh => [qw(STDERR STDOUT)], - pid_file => $cfg->{pid}, + pid_file => $cfg->{pid_file}, ); if (my $pid = $daemon->Init) { _log(_log_fh($cfg->{'log-file'}), "INFO: Forked PID $pid"); @@ -74,11 +75,19 @@ sub go { # In the child process. Set up log file handle and go. $cfg->{log_fh} = _log_fh delete $cfg->{'log-file'}; + $cfg->{pid_file} = delete $cfg->{'pid-file'} if exists $cfg->{'pid-file'}; my $cmd = $class->new( $cfg ); $SIG{TERM} = sub { $cmd->continue(0) }; $cmd->run(@ARGV); } +sub DEMOLISH { + my $self = shift; + if (my $path = $self->pid_file) { + unlink $path; + } +} + sub run { my $self = shift; my $pgxn = PGXN::Manager->instance; @@ -199,7 +208,7 @@ sub _config { \%opts, 'env|E=s', 'daemonize|D', - 'pid=s', + 'pid-file|pid=s', 'log-file|l=s', 'interval|i=s', 'verbose|V+', diff --git a/t/consumer.t b/t/consumer.t index 3decd0b..5defab5 100644 --- a/t/consumer.t +++ b/t/consumer.t @@ -8,11 +8,12 @@ use Encode qw(encode_utf8); use JSON::XS; use File::Temp (); -use Test::More tests => 98; +use Test::More tests => 103; # use Test::More 'no_plan'; use Test::Output; use Test::MockModule; use Test::Exception; +use Test::File; use Test::File::Contents; ############################################################################## @@ -57,7 +58,8 @@ DEFAULTS: { my $consumer = new_ok $CLASS; is $consumer->verbose, 0, 'Default verbosity is 0'; is $consumer->interval, 5, 'Default interval is 5'; - is $consumer->continue, 1, 'Defaault continue is 1'; + is $consumer->continue, 1, 'Default continue is 1'; + is $consumer->pid_file, undef, 'Default pid file is undef'; ok $consumer->log_fh, 'Should have log file handle'; } @@ -97,6 +99,18 @@ sub output { } 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(pid_file => $fn); + file_exists_ok $fn, 'PID file should exist'; + }; + file_not_exists_ok $fn, 'PID file be gone'; +} + ############################################################################## # Load the test environment configuration. sub load_config { @@ -119,7 +133,7 @@ CONFIG: { my $opts = $CLASS->_config; is_deeply $opts, { daemonize => 1, - pid => 'pid.txt', + 'pid-file' => 'pid.txt', interval => 2.2, verbose => 1, 'log-file' => 'log.txt', @@ -195,18 +209,23 @@ LOAD: { ############################################################################## # Test go. DAEMONIZE: { - # Mock Proc::Daemon + my $tmp = File::Temp->new(UNLINK => 0); + my $pid_file = $tmp->filename; + file_exists_ok $pid_file, 'PID file should exist'; + + # Mock Proc::Daemon my $mock_proc = Test::MockModule->new('Proc::Daemon'); $mock_proc->mock(Init => 0); my $mocker = Test::MockModule->new($CLASS); my $ran; $mocker->mock(run => sub { $ran = 1; 0 }); - local @ARGV = qw(--env test --daemonize); + local @ARGV = (qw(--env test --daemonize --pid-file), $pid_file); is $CLASS->go, 0, 'Should get zero from go'; ok $ran, 'Should have run'; is output(), '', 'Should have no output'; ok defined delete $SIG{TERM}, 'Should have set term signal'; is delete $ENV{PLACK_ENV}, 'test', 'Should have set test env'; + file_not_exists_ok $pid_file, 'Should have deleted pid file'; # Now make a pid. $mock_proc->mock(Init => 42); @@ -215,7 +234,7 @@ DAEMONIZE: { stdout_is { is $CLASS->go, 0, 'Should get zero from go' } "$logtime - INFO: Forked PID 42\n", 'Should have emitted PID'; ok !$ran, 'Should not have run'; - is $SIG{TERM}, undef, 'Should not ahve set term signal'; + is $SIG{TERM}, undef, 'Should not have set term signal'; is delete $ENV{PLACK_ENV}, 'development', 'Should have set development env'; $mocker->unmock('run'); }