* [PATCH 0/3] more networking/daemon fixes
@ 2016-05-24 4:22 7% Eric Wong
2016-05-24 4:22 6% ` [PATCH 3/3] git-http-backend: use qspawn to limit running processes Eric Wong
0 siblings, 1 reply; 2+ results
From: Eric Wong @ 2016-05-24 4:22 UTC (permalink / raw)
To: meta
This series finally reinstates process limiting by queueing
processes instead of rejecting and falling back to dumb.
Falling back to dumb fails badly if we have to switch in
the middle of a pack negotiation.
I've tested it a bunch with aborted git clones of large
repositories with our own -httpd (using async) and fixed a few
longstanding bugs. The generic PSGI code still needs work
to resume properly and to avoid leaking references.
Eric Wong (3):
standardize timer-related event-loop code
http: fix various race conditions
git-http-backend: use qspawn to limit running processes
lib/PublicInbox/EvCleanup.pm | 41 ++++++++++++++++++++
lib/PublicInbox/GitHTTPBackend.pm | 38 ++++++++-----------
lib/PublicInbox/HTTP.pm | 80 +++++++++++++++++++--------------------
lib/PublicInbox/HTTPD/Async.pm | 3 +-
lib/PublicInbox/NNTP.pm | 30 +++++++--------
lib/PublicInbox/Qspawn.pm | 52 +++++++++++++++++++++++++
t/qspawn.t | 60 +++++++++++++++++++++++++++++
7 files changed, 223 insertions(+), 81 deletions(-)
^ permalink raw reply [relevance 7%]
* [PATCH 3/3] git-http-backend: use qspawn to limit running processes
2016-05-24 4:22 7% [PATCH 0/3] more networking/daemon fixes Eric Wong
@ 2016-05-24 4:22 6% ` Eric Wong
0 siblings, 0 replies; 2+ results
From: Eric Wong @ 2016-05-24 4:22 UTC (permalink / raw)
To: meta
Having an excessive amount of git-pack-objects processes is
dangerous to the health of the server. Queue up process spawning
for long-running responses and serve them sequentially, instead.
---
lib/PublicInbox/GitHTTPBackend.pm | 38 ++++++++++---------------
lib/PublicInbox/Qspawn.pm | 52 +++++++++++++++++++++++++++++++++
t/qspawn.t | 60 +++++++++++++++++++++++++++++++++++++++
3 files changed, 127 insertions(+), 23 deletions(-)
create mode 100644 lib/PublicInbox/Qspawn.pm
create mode 100644 t/qspawn.t
diff --git a/lib/PublicInbox/GitHTTPBackend.pm b/lib/PublicInbox/GitHTTPBackend.pm
index ded56b3..9464cb4 100644
--- a/lib/PublicInbox/GitHTTPBackend.pm
+++ b/lib/PublicInbox/GitHTTPBackend.pm
@@ -8,9 +8,9 @@ use strict;
use warnings;
use Fcntl qw(:seek);
use IO::File;
-use PublicInbox::Spawn qw(spawn);
use HTTP::Date qw(time2str);
use HTTP::Status qw(status_message);
+use PublicInbox::Qspawn;
# n.b. serving "description" and "cloneurl" should be innocuous enough to
# not cause problems. serving "config" might...
@@ -167,11 +167,6 @@ sub serve_smart {
unless (defined $fd && $fd >= 0) {
$in = input_to_file($env) or return r(500);
}
- my ($rpipe, $wpipe);
- unless (pipe($rpipe, $wpipe)) {
- err($env, "error creating pipe: $! - going static");
- return;
- }
my %env = %ENV;
# GIT_COMMITTER_NAME, GIT_COMMITTER_EMAIL
# may be set in the server-process and are passed as-is
@@ -187,20 +182,13 @@ sub serve_smart {
my $git_dir = $git->{git_dir};
$env{GIT_HTTP_EXPORT_ALL} = '1';
$env{PATH_TRANSLATED} = "$git_dir/$path";
- my %rdr = ( 0 => fileno($in), 1 => fileno($wpipe) );
- my $pid = spawn([qw(git http-backend)], \%env, \%rdr);
- unless (defined $pid) {
- err($env, "error spawning: $! - going static");
- return;
- }
- $wpipe = $in = undef;
- my $fh;
+ my %rdr = ( 0 => fileno($in) );
+ my $x = PublicInbox::Qspawn->new([qw(git http-backend)], \%env, \%rdr);
+ my ($fh, $rpipe);
my $end = sub {
$rpipe = undef;
- my $e = $pid == waitpid($pid, 0) ?
- $? : "PID:$pid still running?";
- if ($e) {
- err($env, "git http-backend ($git_dir): $e");
+ if (my $err = $x->finish) {
+ err($env, "git http-backend ($git_dir): $err");
drop_client($env);
}
$fh->close if $fh; # async-only
@@ -248,11 +236,15 @@ sub serve_smart {
# holding the input here is a waste of FDs and memory
$env->{'psgi.input'} = undef;
- if ($async) {
- $async = $async->($rpipe, $cb, $end);
- } else { # generic PSGI
- $cb->() while $rd_hdr;
- }
+ $x->start(sub { # may run later, much later...
+ ($rpipe) = @_;
+ $in = undef;
+ if ($async) {
+ $async = $async->($rpipe, $cb, $end);
+ } else { # generic PSGI
+ $cb->() while $rd_hdr;
+ }
+ });
};
}
diff --git a/lib/PublicInbox/Qspawn.pm b/lib/PublicInbox/Qspawn.pm
new file mode 100644
index 0000000..9e4c8e0
--- /dev/null
+++ b/lib/PublicInbox/Qspawn.pm
@@ -0,0 +1,52 @@
+# Copyright (C) 2016 all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+package PublicInbox::Qspawn;
+use strict;
+use warnings;
+use PublicInbox::Spawn qw(popen_rd);
+our $LIMIT = 1;
+my $running = 0;
+my @run_queue;
+
+sub new ($$$;) {
+ my ($class, $cmd, $env, $opt) = @_;
+ bless { args => [ $cmd, $env, $opt ] }, $class;
+}
+
+sub _do_spawn {
+ my ($self, $cb) = @_;
+ my $err;
+ ($self->{rpipe}, $self->{pid}) = popen_rd(@{$self->{args}});
+ if ($self->{pid}) {
+ $running++;
+ } else {
+ $self->{err} = $!;
+ }
+ $cb->($self->{rpipe});
+}
+
+sub finish ($) {
+ my ($self) = @_;
+ if (delete $self->{rpipe}) {
+ my $pid = delete $self->{pid};
+ $self->{err} = $pid == waitpid($pid, 0) ? $? :
+ "PID:$pid still running?";
+ $running--;
+ }
+ if (my $next = shift @run_queue) {
+ _do_spawn(@$next);
+ }
+ $self->{err};
+}
+
+sub start ($$) {
+ my ($self, $cb) = @_;
+
+ if ($running < $LIMIT) {
+ _do_spawn($self, $cb);
+ } else {
+ push @run_queue, [ $self, $cb ];
+ }
+}
+
+1;
diff --git a/t/qspawn.t b/t/qspawn.t
new file mode 100644
index 0000000..05072e2
--- /dev/null
+++ b/t/qspawn.t
@@ -0,0 +1,60 @@
+# Copyright (C) 2016 all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+use Test::More;
+use_ok 'PublicInbox::Qspawn';
+{
+ my $x = PublicInbox::Qspawn->new([qw(true)]);
+ my $run = 0;
+ $x->start(sub {
+ my ($rpipe) = @_;
+ is(0, sysread($rpipe, my $buf, 1), 'read zero bytes');
+ ok(!$x->finish, 'no error on finish');
+ $run = 1;
+ });
+ is($run, 1, 'callback ran alright');
+}
+
+{
+ my $x = PublicInbox::Qspawn->new([qw(false)]);
+ my $run = 0;
+ $x->start(sub {
+ my ($rpipe) = @_;
+ is(0, sysread($rpipe, my $buf, 1), 'read zero bytes from false');
+ my $err = $x->finish;
+ is($err, 256, 'error on finish');
+ $run = 1;
+ });
+ is($run, 1, 'callback ran alright');
+}
+
+foreach my $cmd ([qw(sleep 1)], [qw(sh -c), 'sleep 1; false']) {
+ my $s = PublicInbox::Qspawn->new($cmd);
+ my @run;
+ $s->start(sub {
+ my ($rpipe) = @_;
+ push @run, 'sleep';
+ is(0, sysread($rpipe, my $buf, 1), 'read zero bytes');
+ });
+ my $n = 0;
+ my @t = map {
+ my $i = $n++;
+ my $x = PublicInbox::Qspawn->new([qw(true)]);
+ $x->start(sub {
+ my ($rpipe) = @_;
+ push @run, $i;
+ });
+ [$x, $i]
+ } (0..2);
+
+ if ($cmd->[-1] =~ /false\z/) {
+ ok($s->finish, 'got error on false after sleep');
+ } else {
+ ok(!$s->finish, 'no error on sleep');
+ }
+ ok(!$_->[0]->finish, "true $_->[1] succeeded") foreach @t;
+ is_deeply([qw(sleep 0 1 2)], \@run, 'ran in order');
+}
+
+done_testing();
+
+1;
^ permalink raw reply related [relevance 6%]
Results 1-2 of 2 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2016-05-24 4:22 7% [PATCH 0/3] more networking/daemon fixes Eric Wong
2016-05-24 4:22 6% ` [PATCH 3/3] git-http-backend: use qspawn to limit running processes Eric Wong
Code repositories for project(s) associated with this public inbox
https://80x24.org/public-inbox.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).