INSTALL | 11 ++++++----- Makefile.PL | 4 ---- lib/PublicInbox/Daemon.pm | 3 ++- lib/PublicInbox/HTTP.pm | 1 + lib/PublicInbox/Inbox.pm | 45 ++++++++++++++++++++++++++++++++------------- t/config_limiter.t | 1 - diff --git a/INSTALL b/INSTALL index 313a295151883ed580ea5dac7186b95c40053402..834376278bc216da76a586a1c88dd02028ba729f 100644 --- a/INSTALL +++ b/INSTALL @@ -36,11 +36,6 @@ * Date::Parse deb: libtimedate-perl pkg: p5-TimeDate rpm: perl-TimeDate -* Devel::Peek deb: libperl5.$MINOR (e.g. libperl5.24) - pkg: perl5 - rpm: perl-Devel-Peek - (typically installed alongside Perl5) - * Email::MIME deb: libemail-mime-perl pkg: p5-Email-MIME rpm: perl-Email-MIME @@ -124,6 +119,12 @@ - DBI deb: libdbi-perl pkg: p5-DBI rpm: perl-DBI (pulled in by DBD::SQLite) + +* Devel::Peek deb: libperl5.$MINOR (e.g. libperl5.24) + pkg: perl5 + rpm: perl-Devel-Peek + (optional for stale FD cleanup in daemons, + typically installed alongside Perl5) - Filesys::Notify::Simple deb: libfilesys-notify-simple-perl pkg: pkg-Filesys-Notify-Simple diff --git a/Makefile.PL b/Makefile.PL index 6be913b168f58cf4dd7a86bb67ab1dcb5cf5f74d..de0c49fd8e2c5660c08f913cadd1371e4b7e7116 100644 --- a/Makefile.PL +++ b/Makefile.PL @@ -38,10 +38,6 @@ # libperl$PERL_VERSION or libencode-perl on Debian, # `perl5' on FreeBSD 'Encode' => 0, - # libperl$PERL_VERSION on Debian, `perl5' on FreeBSD, - # but Fedora seems to need this separately - 'Devel::Peek' => 0, - # TODO: these should really be made optional... 'Plack' => 0, 'URI::Escape' => 0, diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm index 68ba987636524e8bba58e4c5e1535f2f4de32e55..227ba5f979d09ef23eebaf892284b27137222165 100644 --- a/lib/PublicInbox/Daemon.pm +++ b/lib/PublicInbox/Daemon.pm @@ -13,6 +13,7 @@ use Time::HiRes qw(clock_gettime CLOCK_MONOTONIC); STDOUT->autoflush(1); STDERR->autoflush(1); require PublicInbox::DS; +require PublicInbox::EvCleanup; require POSIX; require PublicInbox::Listener; require PublicInbox::ParentPipe; @@ -463,6 +464,7 @@ } sub daemon_loop ($$) { my ($refresh, $post_accept) = @_; + PublicInbox::EvCleanup::enable(); # early for $refresh my $parent_pipe; if ($worker_processes > 0) { $refresh->(); # preload by default @@ -485,7 +487,6 @@ # this calls epoll_create: @listeners = map { PublicInbox::Listener->new($_, $post_accept) } @listeners; - PublicInbox::EvCleanup::enable(); PublicInbox::DS->EventLoop; $parent_pipe = undef; } diff --git a/lib/PublicInbox/HTTP.pm b/lib/PublicInbox/HTTP.pm index 11bd241e91a1661d316a7f3d037f047b938762df..10e6d6a43b518115c7eece8a5f11e67ebe613fb2 100644 --- a/lib/PublicInbox/HTTP.pm +++ b/lib/PublicInbox/HTTP.pm @@ -18,6 +18,7 @@ use Plack::HTTPParser qw(parse_http_request); # XS or pure Perl use HTTP::Status qw(status_message); use HTTP::Date qw(time2str); use IO::Handle; +require PublicInbox::EvCleanup; use constant { CHUNK_START => -1, # [a-f0-9]+\r\n CHUNK_END => -2, # \r\n diff --git a/lib/PublicInbox/Inbox.pm b/lib/PublicInbox/Inbox.pm index 76aeefbdd0e4b4c2c0d1d0bdc4353fd3d2e54d08..0b118b28cea84418f18a73cd5f6a63cfbbe0dae9 100644 --- a/lib/PublicInbox/Inbox.pm +++ b/lib/PublicInbox/Inbox.pm @@ -7,25 +7,30 @@ use strict; use warnings; use PublicInbox::Git; use PublicInbox::MID qw(mid2path); -use Devel::Peek qw(SvREFCNT); use PublicInbox::MIME; +# Long-running "git-cat-file --batch" processes won't notice +# unlinked packs, so we need to restart those processes occasionally. +# Xapian and SQLite file handles are mostly stable, but sometimes an +# admin will attempt to replace them atomically after compact/vacuum +# and we need to be prepared for that. my $cleanup_timer; -eval { - $cleanup_timer = 'disabled'; - require PublicInbox::EvCleanup; - $cleanup_timer = undef; # OK if we get here -}; -my $cleanup_broken = $@; - +my $cleanup_avail = -1; # 0, or 1 +my $have_devel_peek; my $CLEANUP = {}; # string(inbox) -> inbox sub cleanup_task () { $cleanup_timer = undef; my $next = {}; for my $ibx (values %$CLEANUP) { my $again; - foreach my $f (qw(mm search)) { - delete $ibx->{$f} if SvREFCNT($ibx->{$f}) == 1; + if ($have_devel_peek) { + foreach my $f (qw(mm search)) { + # we bump refcnt by assigning tmp, here: + my $tmp = $ibx->{$f} or next; + next if Devel::Peek::SvREFCNT($tmp) > 2; + delete $ibx->{$f}; + # refcnt is zero when tmp is out-of-scope + } } my $expire = time - 60; if (my $git = $ibx->{git}) { @@ -36,16 +41,30 @@ foreach my $git (@$gits) { $again = 1 if $git->cleanup($expire); } } - $again ||= !!($ibx->{mm} || $ibx->{search}); + if ($have_devel_peek) { + $again ||= !!($ibx->{mm} || $ibx->{search}); + } $next->{"$ibx"} = $ibx if $again; } $CLEANUP = $next; } +sub cleanup_possible () { + # no need to require EvCleanup, here, if it were enabled another + # module would've require'd it, already + eval { PublicInbox::EvCleanup::enabled() } or return 0; + + eval { + require Devel::Peek; # needs separate package in Fedora + $have_devel_peek = 1; + }; + 1; +} + sub _cleanup_later ($) { my ($self) = @_; - return if $cleanup_broken; - return unless PublicInbox::EvCleanup::enabled(); + $cleanup_avail = cleanup_possible() if $cleanup_avail < 0; + return if $cleanup_avail != 1; $cleanup_timer ||= PublicInbox::EvCleanup::later(*cleanup_task); $CLEANUP->{"$self"} = $self; } diff --git a/t/config_limiter.t b/t/config_limiter.t index 85a71257b7544d33e1e138b82e0689c92782f11f..b18951a61a2f5c8f4f664fd7f80078b44fb8a26e 100644 --- a/t/config_limiter.t +++ b/t/config_limiter.t @@ -38,7 +38,6 @@ my $lim = $git->{-httpbackend_limiter}; ok($lim, 'Limiter exists'); is($lim->{max}, 3, 'limiter has expected slots'); $ibx->{git} = undef; - PublicInbox::Inbox::cleanup_task; my $new = $ibx->git; isnt($old, "$new", 'got new Git object'); is("$new->{-httpbackend_limiter}", "$lim", 'same limiter');