Re: [PATCH 1/9] gitweb: Load checking
- From
Jakub Narebski <jnareb@gmail.com>
- Date
- Jan 15, 2010, 22:30 UTC
- Message-ID
- <m33a27dmsk.fsf@localhost.localdomain>
- In-Reply-To
- <1263432185-21334-2-git-send-email-warthog9@eaglescrag.net>
This one looks good, but while examining other patch in this series I have noticed rare situation where we would get Perl error with this patch.
I have added fix-up for this issue, although I guess that better solution might be not to add any <script> element for git_footer_html called from die_error.
I'm sorry I haven't noticed this earlier.
"John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes:
Show 19 quoted lines
> From: John 'Warthog9' Hawley <warthog9@kernel.org> > > This changes slightly the behavior 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. > > This behavior is controlled by $maxload configuration variable. > Default is a load of 300, which for most cases should never be hit. > Unset it (set it to undefined value, i.e. undef) to turn off checking. > > Currently it requires that '/proc/loadavg' file exists, otherwise the > load check is bypassed (load is taken to be 0). So platforms that do > not implement '/proc/loadavg' currently cannot use this feature. > (provisions are included for additional checks to be added by others) >
While at it check that $action is defined before comparing it in git_footer_html() subroutine. Until this patch there were no direct or indirect (via die_error) invocation of git_footer_html() with $action undefined; each call was after dispatch, which sets $action to default value if it is undefined.
This would cause Perl error ("Use of uninitialized value in string eq")
if load is too high _and_ gitweb was invoked without action parameter
explicitly set (e.g. for projects list).
Show 106 quoted lines
> Signed-off-by: John 'Warthog9' Hawley <warthog9@kernel.org>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
> gitweb/README | 7 ++++++-
> gitweb/gitweb.perl | 45 +++++++++++++++++++++++++++++++++++++++++----
> 2 files changed, 47 insertions(+), 5 deletions(-)
>
> diff --git a/gitweb/README b/gitweb/README
> index e34ee79..6c2c8e1 100644
> --- a/gitweb/README
> +++ b/gitweb/README
> @@ -174,7 +174,7 @@ not include variables usually directly set during build):
> Base URL for relative URLs in pages generated by gitweb,
> (e.g. $logo, $favicon, @stylesheets if they are relative URLs),
> needed and used only for URLs with nonempty PATH_INFO via
> - <base href="$base_url>. Usually gitweb sets its value correctly,
> + <base href="$base_url">. Usually gitweb sets its value correctly,
> and there is no need to set this variable, e.g. to $my_uri or "/".
> * $home_link
> Target of the home link on top of all pages (the first part of view
> @@ -228,6 +228,11 @@ not include variables usually directly set during build):
> repositories from launching cross-site scripting (XSS) attacks. Set this
> to true if you don't trust the content of your repositories. The default
> is false.
> + * $maxload
> + Used to set the maximum load that we will still respond to gitweb queries.
> + If server load exceed this value then return "503 Service Unavaliable" error.
> + Server load is taken to be 0 if gitweb cannot determine its value. Set it to
> + undefined value to turn it off. The default is 300.
>
>
> Projects list file format
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 7e477af..0a07d3a 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -221,6 +221,12 @@ our %avatar_size = (
> 'double' => 32
> );
>
> +# 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.
> +# If gitweb cannot determined server load, it is taken to be 0.
> +# Leave it undefined (or set to 'undef') to turn off load checking.
> +our $maxload = 300;
> +
> # You define site-wide feature defaults here; override them with
> # $GITWEB_CONFIG as necessary.
> our %feature = (
> @@ -551,6 +557,32 @@ if (-e $GITWEB_CONFIG) {
> do $GITWEB_CONFIG_SYSTEM if -e $GITWEB_CONFIG_SYSTEM;
> }
>
> +# Get loadavg of system, to compare against $maxload.
> +# Currently it requires '/proc/loadavg' present to get loadavg;
> +# if it is not present it returns 0, which means no load checking.
> +sub get_loadavg {
> + if( -e '/proc/loadavg' ){
> + open my $fd, '<', '/proc/loadavg'
> + or return 0;
> + my @load = split(/\s+/, scalar <$fd>);
> + close $fd;
> +
> + # The first three columns measure CPU and IO utilization of the last one,
> + # five, and 10 minute periods. The fourth column shows the number of
> + # currently running processes and the total number of processes in the m/n
> + # format. The last column displays the last process ID used.
> + return $load[0] || 0;
> + }
> + # additional checks for load average should go here for things that don't export
> + # /proc/loadavg
> +
> + return 0;
> +}
> +
> +if (defined $maxload && get_loadavg() > $maxload) {
> + die_error(503, "The load average on the server is too high");
> +}
> +
> # version of the core git binary
> our $git_version = qx("$GIT" --version) =~ m/git version (.*)$/ ? $1 : "unknown";
> $number_of_git_cmds++;
> @@ -3354,14 +3386,19 @@ sub git_footer_html {
> # 500: The server isn't configured properly, or
> # an internal error occurred (e.g. failed assertions caused by bugs), or
> # an unknown error occurred (e.g. the git binary died unexpectedly).
> +# 503: The server is currently unavailable (because it is overloaded,
> +# or down for maintenance). Generally, this is a temporary state.
> sub die_error {
> my $status = shift || 500;
> my $error = shift || "Internal server error";
>
> - my %http_responses = (400 => '400 Bad Request',
> - 403 => '403 Forbidden',
> - 404 => '404 Not Found',
> - 500 => '500 Internal Server Error');
> + my %http_responses = (
> + 400 => '400 Bad Request',
> + 403 => '403 Forbidden',
> + 404 => '404 Not Found',
> + 500 => '500 Internal Server Error',
> + 503 => '503 Service Unavailable',
> + );
> git_header_html($http_responses{$status});
> print <<EOF;
> <div class="page_body">@@ -3354,7 +3354,8 @@ sub git_footer_html { } print qq!<script type="text/javascript" src="$javascript"></script>\n!; - if ($action eq 'blame_incremental') { + if (defined $action && + $action eq 'blame_incremental') { print qq!<script type="text/javascript">\n!. qq!startBlame("!. href(action=>"blame_data", -replay=>1) .qq!",\n!. qq! "!. href() .qq!");\n!. > -- > 1.6.5.2 >
-- Jakub Narebski Poland ShadeHawk on #git