user/dev discussion of public-inbox itself
 help / color / Atom feed
* [PATCH 0/2] git: a few minor cleanup tweaks to daemons
@ 2019-06-01  3:37 Eric Wong
  2019-06-01  3:37 ` [PATCH 1/2] git: unconditional expiry Eric Wong
  2019-06-01  3:37 ` [PATCH 2/2] git: drop the deleted err_c file Eric Wong
  0 siblings, 2 replies; 3+ messages in thread
From: Eric Wong @ 2019-06-01  3:37 UTC (permalink / raw)
  To: meta

These probably don't affect anybody running servers atm, as I'm
probably the most hardware-deficient of public-inbox admins out
there...

Eric Wong (2):
  git: unconditional expiry
  git: drop the deleted err_c file

 lib/PublicInbox/Git.pm   | 21 ++++++---------------
 lib/PublicInbox/Inbox.pm |  5 ++---
 t/git.t                  |  3 +--
 3 files changed, 9 insertions(+), 20 deletions(-)

-- 
EW


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 1/2] git: unconditional expiry
  2019-06-01  3:37 [PATCH 0/2] git: a few minor cleanup tweaks to daemons Eric Wong
@ 2019-06-01  3:37 ` Eric Wong
  2019-06-01  3:37 ` [PATCH 2/2] git: drop the deleted err_c file Eric Wong
  1 sibling, 0 replies; 3+ messages in thread
From: Eric Wong @ 2019-06-01  3:37 UTC (permalink / raw)
  To: meta

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 2/2] git: drop the deleted err_c file
  2019-06-01  3:37 [PATCH 0/2] git: a few minor cleanup tweaks to daemons Eric Wong
  2019-06-01  3:37 ` [PATCH 1/2] git: unconditional expiry Eric Wong
@ 2019-06-01  3:37 ` Eric Wong
  1 sibling, 0 replies; 3+ messages in thread
From: Eric Wong @ 2019-06-01  3:37 UTC (permalink / raw)
  To: meta

No reason to leave that (usually) empty file open after killing off
"cat-file --batch-check".  This wasn't an unbound leak, though,
as respawning the --batch-check process would've clobbered the
old err_c file.
---
 lib/PublicInbox/Git.pm | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index 9a38d7c..9014e02 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -211,9 +211,10 @@ sub check {
 }
 
 sub _destroy {
-	my ($self, $in, $out, $pid) = @_;
+	my ($self, $in, $out, $pid, $err) = @_;
 	my $p = delete $self->{$pid} or return;
 	delete @$self{($in, $out)};
+	delete $self->{$err} if $err; # `err_c'
 	waitpid $p, 0;
 }
 
@@ -243,7 +244,7 @@ sub qx {
 sub cleanup {
 	my ($self) = @_;
 	_destroy($self, qw(in out pid));
-	_destroy($self, qw(in_c out_c pid_c));
+	_destroy($self, qw(in_c out_c pid_c err_c));
 	!!($self->{pid} || $self->{pid_c});
 }
 
-- 
EW


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-01  3:37 [PATCH 0/2] git: a few minor cleanup tweaks to daemons Eric Wong
2019-06-01  3:37 ` [PATCH 1/2] git: unconditional expiry Eric Wong
2019-06-01  3:37 ` [PATCH 2/2] git: drop the deleted err_c file Eric Wong

user/dev discussion of public-inbox itself

Archives are clonable:
	git clone --mirror https://public-inbox.org/meta
	git clone --mirror http://czquwvybam4bgbro.onion/meta
	git clone --mirror http://hjrcffqmbrq6wope.onion/meta
	git clone --mirror http://ou63pmih66umazou.onion/meta

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.mail.public-inbox.meta
	nntp://ou63pmih66umazou.onion/inbox.comp.mail.public-inbox.meta
	nntp://czquwvybam4bgbro.onion/inbox.comp.mail.public-inbox.meta
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.mail.public-inbox.meta
	nntp://news.gmane.org/gmane.mail.public-inbox.general

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox