git: unconditional expiry

A constant stream of traffic to either httpd/nntpd would mean
git-cat-file processes never expire.  Things can go bad after a
full repack, as a full repack will unlink old pack indices and
git-cat-file does not currently detect unlinked files.

We could do something complicated by recursively stat-ing
objects/pack of every git directory and alternate;
but that's probably not worth the trouble compared to
occasionally restarting the cat-file process.

So simplify the code and let httpd/nntpd expire them
periodically, since spawning a "git-cat-file --batch" process
isn't too expensive.  We already spawn for every request which
hits git-http-backend, cgit, and git-apply.

In the future, we may optionally support the Git::Raw module
to avoid IPC; but we must remain careful to not leave lingering
FDs open to unlinked files after repack.

diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index a4daaa48f0914f538e1232b66267384a2dafec0d..9a38d7c8dce83bc0aa13d0ee870ab33924390580 100644 (file)
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -211,19 +211,9 @@ sub check {
 }
 
 sub _destroy {
-       my ($self, $in, $out, $pid, $expire) = @_;
-       my $rfh = $self->{$in} or return;
-       if (defined $expire) {
-               # at least FreeBSD 11.2 and Linux 4.20 update mtime of the
-               # read end of a pipe when the pipe is written to; dunno
-               # about other OSes.
-               my $mtime = (stat($rfh))[9];
-               return if $mtime > $expire;
-       }
+       my ($self, $in, $out, $pid) = @_;
        my $p = delete $self->{$pid} or return;
-       foreach my $f ($in, $out) {
-               delete $self->{$f};
-       }
+       delete @$self{($in, $out)};
        waitpid $p, 0;
 }
 
@@ -251,9 +241,9 @@ sub qx {
 
 # returns true if there are pending "git cat-file" processes
 sub cleanup {
-       my ($self, $expire) = @_;
-       _destroy($self, qw(in out pid), $expire);
-       _destroy($self, qw(in_c out_c pid_c), $expire);
+       my ($self) = @_;
+       _destroy($self, qw(in out pid));
+       _destroy($self, qw(in_c out_c pid_c));
        !!($self->{pid} || $self->{pid_c});
 }
 

diff --git a/lib/PublicInbox/Inbox.pm b/lib/PublicInbox/Inbox.pm
index 2771a241aa5361f3067ef15f22a1463c4d6b75dc..b3178b987a7a031dc96d134fa7be13423fda1866 100644 (file)
--- a/lib/PublicInbox/Inbox.pm
+++ b/lib/PublicInbox/Inbox.pm
@@ -32,13 +32,12 @@ sub cleanup_task () {
                                # refcnt is zero when tmp is out-of-scope
                        }
                }
-               my $expire = time - 60;
                if (my $git = $ibx->{git}) {
-                       $again = $git->cleanup($expire);
+                       $again = $git->cleanup;
                }
                if (my $gits = $ibx->{-repo_objs}) {
                        foreach my $git (@$gits) {
-                               $again = 1 if $git->cleanup($expire);
+                               $again = 1 if $git->cleanup;
                        }
                }
                if ($have_devel_peek) {

diff --git a/t/git.t b/t/git.t
index 7edf82b44a6dd124cee85e89a1a5f219313a85dc..913f6e5e8928850e8f4c0ce1c2830db4200e0127 100644 (file)
--- a/t/git.t
+++ b/t/git.t
@@ -143,8 +143,7 @@ if ('alternates reloaded') {
        my $config = eval { local $/; <$fh> };
        is($$found, $config, 'alternates reloaded');
 
-       ok($gcf->cleanup(time - 30), 'cleanup did not expire');
-       ok(!$gcf->cleanup(time + 30), 'cleanup can expire');
+       ok(!$gcf->cleanup, 'cleanup can expire');
        ok(!$gcf->cleanup, 'cleanup idempotent');
 
        my $t = $gcf->modified;