* [PATCH 1/2] githttpbackend: fall back to dumb if smart HTTP is off
2016-04-28 1:56 [PATCH 0/2] githttpbackend: dumb HTTP fallbacks Eric Wong
@ 2016-04-28 1:56 ` Eric Wong
2016-04-28 1:56 ` [PATCH 2/2] githttpbackend: clamp to one smart HTTP request at-a-time Eric Wong
1 sibling, 0 replies; 3+ messages in thread
From: Eric Wong @ 2016-04-28 1:56 UTC (permalink / raw)
To: meta
Using http.getanyfile still keeps the http-backend process
alive, so it's better to break out of that process and
handle serving entirely within the HTTP server.
---
lib/PublicInbox/GitHTTPBackend.pm | 40 ++++++++++++++++++++++++---------------
t/httpd.t | 9 +++++++++
2 files changed, 34 insertions(+), 15 deletions(-)
diff --git a/lib/PublicInbox/GitHTTPBackend.pm b/lib/PublicInbox/GitHTTPBackend.pm
index 2c81d4c..c44c67d 100644
--- a/lib/PublicInbox/GitHTTPBackend.pm
+++ b/lib/PublicInbox/GitHTTPBackend.pm
@@ -37,6 +37,12 @@ sub serve {
return $ok if $ok;
}
+ serve_dumb($cgi, $git, $path);
+}
+
+sub serve_dumb {
+ my ($cgi, $git, $path) = @_;
+
my $type;
if ($path =~ /\A(?:$BIN)\z/o) {
$type = 'application/octet-stream';
@@ -141,11 +147,11 @@ sub serve_smart {
}
my ($rpipe, $wpipe);
unless (pipe($rpipe, $wpipe)) {
- $err->print("error creating pipe: $!\n");
- return r(500);
+ $err->print("error creating pipe: $! - going static\n");
+ return;
}
my %env = %ENV;
- # GIT_HTTP_EXPORT_ALL, GIT_COMMITTER_NAME, GIT_COMMITTER_EMAIL
+ # GIT_COMMITTER_NAME, GIT_COMMITTER_EMAIL
# may be set in the server-process and are passed as-is
foreach my $name (qw(QUERY_STRING
REMOTE_USER REMOTE_ADDR
@@ -162,8 +168,8 @@ sub serve_smart {
my %rdr = ( 0 => fileno($in), 1 => fileno($wpipe) );
my $pid = spawn([qw(git http-backend)], \%env, \%rdr);
unless (defined $pid) {
- $err->print("error spawning: $!\n");
- return r(500);
+ $err->print("error spawning: $! - going static\n");
+ return;
}
$wpipe = $in = undef;
$buf = '';
@@ -172,19 +178,19 @@ sub serve_smart {
if ($fh) {
$fh->close;
$fh = undef;
- } else {
- $res->(r(500)) if $res;
}
if ($rpipe) {
$rpipe->close; # _may_ be Danga::Socket::close
$rpipe = undef;
}
- if (defined $pid) {
- my $wpid = $pid;
- $pid = undef;
- return if $wpid == waitpid($wpid, 0);
+ if (defined $pid && $pid != waitpid($pid, 0)) {
$err->print("git http-backend ($git_dir): $?\n");
+ } else {
+ $pid = undef;
}
+ return unless $res;
+ my $dumb = serve_dumb($cgi, $git, $path);
+ ref($dumb) eq 'ARRAY' ? $res->($dumb) : $dumb->($res);
};
my $fail = sub {
my ($e) = @_;
@@ -215,10 +221,14 @@ sub serve_smart {
push @h, $k, $v;
}
}
- # write response header:
- $fh = $res->([ $code, \@h ]);
- $res = undef;
- $fh->write($buf);
+ if ($code == 403) {
+ # smart cloning disabled, serve dumbly
+ # in $end since we never undef $res in here
+ } else { # write response header:
+ $fh = $res->([ $code, \@h ]);
+ $res = undef;
+ $fh->write($buf);
+ }
$buf = '';
} # else { keep reading ... }
};
diff --git a/t/httpd.t b/t/httpd.t
index 28f507d..0379031 100644
--- a/t/httpd.t
+++ b/t/httpd.t
@@ -105,6 +105,15 @@ EOF
is(system(qw(git clone -q --mirror),
"http://$host:$port/$group", "$tmpdir/clone.git"),
0, 'clone successful');
+
+ # ensure dumb cloning works, too:
+ is(system('git', "--git-dir=$maindir",
+ qw(config http.uploadpack false)),
+ 0, 'disable http.uploadpack');
+ is(system(qw(git clone -q --mirror),
+ "http://$host:$port/$group", "$tmpdir/dumb.git"),
+ 0, 'clone successful');
+
ok(kill('TERM', $pid), 'killed httpd');
$pid = undef;
waitpid(-1, 0);
--
EW
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH 2/2] githttpbackend: clamp to one smart HTTP request at-a-time
2016-04-28 1:56 [PATCH 0/2] githttpbackend: dumb HTTP fallbacks Eric Wong
2016-04-28 1:56 ` [PATCH 1/2] githttpbackend: fall back to dumb if smart HTTP is off Eric Wong
@ 2016-04-28 1:56 ` Eric Wong
1 sibling, 0 replies; 3+ messages in thread
From: Eric Wong @ 2016-04-28 1:56 UTC (permalink / raw)
To: meta
Server admins may not be able to afford to have too many
git-pack-objects processes running at once. Since PSGI
HTTP servers should already be configured to use multiple
processes for other requests; limit concurrency of smart
backends to one; and fall back to dumb responses if we're
already generating a pack.
---
lib/PublicInbox/GitHTTPBackend.pm | 12 ++++++++++++
t/httpd.t | 2 +-
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/lib/PublicInbox/GitHTTPBackend.pm b/lib/PublicInbox/GitHTTPBackend.pm
index c44c67d..a7cac10 100644
--- a/lib/PublicInbox/GitHTTPBackend.pm
+++ b/lib/PublicInbox/GitHTTPBackend.pm
@@ -10,6 +10,14 @@ use Fcntl qw(:seek);
use IO::File;
use PublicInbox::Spawn qw(spawn);
+# TODO: make configurable, but keep in mind it's better to have
+# multiple -httpd worker processes which are already scaled to
+# the proper number of CPUs and memory. git-pack-objects(1) may
+# also use threads and bust memory limits, too, so I recommend
+# limiting threads to 1 (via `pack.threads` knob in git) for serving.
+my $LIMIT = 1;
+my $nr_running = 0;
+
# n.b. serving "description" and "cloneurl" should be innocuous enough to
# not cause problems. serving "config" might...
my @text = qw[HEAD info/refs
@@ -31,6 +39,8 @@ sub r {
sub serve {
my ($cgi, $git, $path) = @_;
+ return serve_dumb($cgi, $git, $path) if $nr_running >= $LIMIT;
+
my $service = $cgi->param('service') || '';
if ($service =~ /\Agit-\w+-pack\z/ || $path =~ /\Agit-\w+-pack\z/) {
my $ok = serve_smart($cgi, $git, $path);
@@ -174,6 +184,7 @@ sub serve_smart {
$wpipe = $in = undef;
$buf = '';
my ($vin, $fh, $res);
+ $nr_running++;
my $end = sub {
if ($fh) {
$fh->close;
@@ -182,6 +193,7 @@ sub serve_smart {
if ($rpipe) {
$rpipe->close; # _may_ be Danga::Socket::close
$rpipe = undef;
+ $nr_running--;
}
if (defined $pid && $pid != waitpid($pid, 0)) {
$err->print("git http-backend ($git_dir): $?\n");
diff --git a/t/httpd.t b/t/httpd.t
index 0379031..781fe03 100644
--- a/t/httpd.t
+++ b/t/httpd.t
@@ -104,7 +104,7 @@ EOF
is(system(qw(git clone -q --mirror),
"http://$host:$port/$group", "$tmpdir/clone.git"),
- 0, 'clone successful');
+ 0, 'smart clone successful');
# ensure dumb cloning works, too:
is(system('git', "--git-dir=$maindir",
--
EW
^ permalink raw reply related [flat|nested] 3+ messages in thread