From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS43350 77.247.176.0/21 X-Spam-Status: No, score=-1.5 required=3.0 tests=BAYES_00,RCVD_IN_XBL,SPF_FAIL, SPF_HELO_FAIL shortcircuit=no autolearn=no version=3.3.2 X-Original-To: meta@public-inbox.org Received: from 80x24.org (politkovskaja.torservers.net [77.247.181.165]) by dcvr.yhbt.net (Postfix) with ESMTP id 16F7920A48 for ; Thu, 28 Apr 2016 01:56:17 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 2/2] githttpbackend: clamp to one smart HTTP request at-a-time Date: Thu, 28 Apr 2016 01:56:08 +0000 Message-Id: <20160428015608.23091-3-e@80x24.org> In-Reply-To: <20160428015608.23091-1-e@80x24.org> References: <20160428015608.23091-1-e@80x24.org> List-Id: 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