From: Jakub Narebski <jnareb@gmail•com>
To: "John 'Warthog9' Hawley" <warthog9@kernel•org>
Cc: git@vger•kernel.org, "John 'Warthog9' Hawley" <warthog9@eaglescrag•net>
Subject: Re: [PATCH 1/6] GITWEB - Load Checking
Date: Thu, 10 Dec 2009 16:52:22 -0800 (PST) [thread overview]
Message-ID: <m34onye3h8.fsf@localhost.localdomain> (raw)
In-Reply-To: <1260488743-25855-2-git-send-email-warthog9@kernel.org>
"John 'Warthog9' Hawley" <warthog9@kernel•org> writes:
> This changes the behavior, slightly, of gitweb so that it verifies
> that the box isn't inundated with before attempting to serve gitweb.
> If the box is overloaded, it basically returns a 503 server unavailable
> until the load falls below the defined threshold. This helps dramatically
> if you have a box that's I/O bound, reaches a certain load and you
> don't want gitweb, the I/O hog that it is, increasing the pain the
> server is already undergoing.
>
> adds $maxload configuration variable. Default is a load of 300,
> which for most cases should never be hit.
Your patch doesn't allow for *turning off* this feature. Reasonable
solution would be to use 'undef' or negative number to turn off this
check (this feature).
>
> Please note this makes the assumption that /proc/loadavg exists
> as there is no good way to read load averages on a great number of
> platforms [READ: Windows], or that it's reasonably accurate.
What about MacOS X, or FreeBSD, or OpenSolaris?
You should mention that it is intended that if gitweb cannot read load
average (for example /proc/loadavg does not exist), then the feature
is turned off, i.e. the check always succeeds. Which is reasonable.
>
> Signed-off-by: John 'Warthog9' Hawley <warthog9@eaglescrag•net>
Why signoff is different from author (warthog9@kernel•org)? Why this
email for signoff? Just curious...
> ---
> gitweb/gitweb.perl | 24 ++++++++++++++++++++++++
> 1 files changed, 24 insertions(+), 0 deletions(-)
Please post patches inline, not as attachement.
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 7e477af..813e48f 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -221,6 +221,11 @@ our %avatar_size = (
> 'double' => 32
> );
>
> +# Used to set the maximum load that we will still respond to gitweb queries.
> +# if we exceed this than we do the processing to figure out if there's a mirror
> +# and redirect to it, or to just return 503 server busy
I'd probably say:
+# Used to set the maximum load that we will still respond to gitweb queries.
+# If server load exceed this value then return "503 server busy" error,
+# (it is also possible to redirect to mirror, if it exists, instead).
> +our $maxload = 300;
> +
> # You define site-wide feature defaults here; override them with
> # $GITWEB_CONFIG as necessary.
> our %feature = (
> @@ -551,6 +556,25 @@ if (-e $GITWEB_CONFIG) {
> do $GITWEB_CONFIG_SYSTEM if -e $GITWEB_CONFIG_SYSTEM;
> }
>
> +# loadavg throttle
> +sub get_loadavg() {
> + my $load;
> + my @loads;
> +
> + open($load, '<', '/proc/loadavg') or return 0;
Why not use one of existing CPAN modules: Sys::Info::Device::CPU,
BSD::getloadavg, Sys::CpuLoad?
Style:
+ open (my $load, '<', '/proc/loadavg') or return 0;
and of course no "my $load" at beginning. Also perhaps $fh, or
$loadfh instead of $load? But this is a minor nit.
> + @loads = split(/\s+/, scalar <$load>);
> + close($load);
> + return $loads[0];
> +}
> +
> +if (get_loadavg() > $maxload) {
> + print "Content-Type: text/plain\n";
> + print "Status: 503 Excessive load on server\n";
> + print "\n";
> + print "The load average on the server is too high\n";
> + exit 0;
Why not use die_error subroutine? Is it to have generate absolutely
minimal load, and that is why you do not use die_error(), or even
$cgi->header()?
Wouldn't a better solution be to use here-doc syntax?
+ print <<'EOF';
+Content-Type: text/plain; charset=utf-8
+Status: 503 Excessive load on server
+
+The load average on the server is too high
+EOF
+ exit 0;
> +}
> +
> # version of the core git binary
> our $git_version = qx("$GIT" --version) =~ m/git version (.*)$/ ? $1 : "unknown";
> $number_of_git_cmds++;
--
Jakub Narebski
Poland
ShadeHawk on #git
next prev parent reply other threads:[~2009-12-11 0:52 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-12-10 23:45 [PATCH 0/6] Gitweb caching changes v2 John 'Warthog9' Hawley
2009-12-10 23:45 ` [PATCH 1/6] GITWEB - Load Checking John 'Warthog9' Hawley
2009-12-10 23:45 ` [PATCH 2/6] GITWEB - Missmatching git w/ gitweb John 'Warthog9' Hawley
2009-12-10 23:45 ` [PATCH 3/6] GITWEB - Add git:// link to summary pages John 'Warthog9' Hawley
2009-12-10 23:45 ` [PATCH 4/6] GITWEB - Makefile changes John 'Warthog9' Hawley
[not found] ` <1260488743-25855-6-git-send-email-warthog9@kernel.org>
2009-12-10 23:45 ` [PATCH 6/6] GITWEB - Separate defaults from main file John 'Warthog9' Hawley
2009-12-11 15:46 ` Jakub Narebski
2009-12-11 15:58 ` J.H.
2009-12-11 22:53 ` Jakub Narebski
2009-12-16 1:22 ` Junio C Hamano
2009-12-16 2:00 ` J.H.
2009-12-16 19:52 ` Jakub Narebski
2009-12-16 20:04 ` J.H.
2009-12-16 2:22 ` Jakub Narebski
2009-12-11 14:28 ` [PATCH 4/6] GITWEB - Makefile changes Jakub Narebski
2009-12-11 16:22 ` J.H.
2009-12-11 16:41 ` Jakub Narebski
2009-12-19 13:32 ` [PATCH/RFCv2 4/6] gitweb: Makefile improvements Jakub Narebski
2009-12-11 12:52 ` [PATCH 3/6] GITWEB - Add git:// link to summary pages Johannes Schindelin
2009-12-11 13:44 ` Jakub Narebski
2009-12-18 21:02 ` [PATCHv2 3/6] gitweb: Optionally add "git" links in project list page Jakub Narebski
2009-12-11 10:52 ` [PATCH 2/6] GITWEB - Missmatching git w/ gitweb Jakub Narebski
2009-12-18 19:18 ` [RFC/PATCHv2 2/6] gitweb: Add option to force version match Jakub Narebski
2009-12-11 12:49 ` [PATCH 2/6] GITWEB - Missmatching git w/ gitweb Johannes Schindelin
2009-12-10 23:54 ` [PATCH 1/6] GITWEB - Load Checking Sverre Rabbelier
2009-12-11 0:52 ` Jakub Narebski [this message]
2009-12-11 1:10 ` Junio C Hamano
2009-12-11 2:19 ` J.H.
2009-12-11 2:50 ` Junio C Hamano
2009-12-11 2:58 ` J.H.
2009-12-11 3:07 ` J.H.
2009-12-11 3:09 ` Junio C Hamano
2009-12-11 10:09 ` Jakub Narebski
2009-12-18 16:36 ` [PATCHv2 1/6] gitweb: Load checking Jakub Narebski
2009-12-11 13:53 ` [PATCH 1/6] GITWEB - Load Checking Mihamina Rakotomandimby
2009-12-10 23:53 ` [PATCH 0/6] Gitweb caching changes v2 Sverre Rabbelier
2009-12-11 15:51 ` Jakub Narebski
[not found] ` <4B226D56.7000004@kernel.org>
2009-12-11 18:01 ` Jakub Narebski
2009-12-11 18:26 ` J.H.
2009-12-12 1:37 ` Jakub Narebski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=m34onye3h8.fsf@localhost.localdomain \
--to=jnareb@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=warthog9@eaglescrag$(echo .)net \
--cc=warthog9@kernel$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox