From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 8CB5A1F609 for ; Sat, 1 Jun 2019 03:37:09 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 1/2] git: unconditional expiry Date: Sat, 1 Jun 2019 03:37:05 +0000 Message-Id: <20190601033706.18113-2-e@80x24.org> In-Reply-To: <20190601033706.18113-1-e@80x24.org> References: <20190601033706.18113-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: 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. --- lib/PublicInbox/Git.pm | 20 +++++--------------- lib/PublicInbox/Inbox.pm | 5 ++--- t/git.t | 3 +-- 3 files changed, 8 insertions(+), 20 deletions(-) diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm index a4daaa4..9a38d7c 100644 --- 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 2771a24..b3178b9 100644 --- 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 7edf82b..913f6e5 100644 --- 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; -- EW