* Re: [PATCH 1/7] gitweb: Load checking [not found] ` <1263374828-15342-2-git-send-email-warthog9@eaglescrag.net> @ 2010-01-13 17:18 ` Jakub Narebski [not found] ` <1263374828-15342-3-git-send-email-warthog9@eaglescrag.net> 1 sibling, 0 replies; 3+ messages in thread From: Jakub Narebski @ 2010-01-13 17:18 UTC (permalink / raw) To: git; +Cc: John 'Warthog9' Hawley On Wed, 13 Jan 2010, John 'Warthog9' Hawley wrote: > > v3 - warthog9: small additional adjustment to indicate where new load > checking should be added for future improvements > v2 - Jakub: switching to using "switching to using" what? > v1 - warthog9: Initial creation > > Signed-off-by: John 'Warthog9' Hawley <warthog9@kernel.org> > Signed-off-by: Jakub Narebski <jnareb@gmail.com> > --- Also, such comments should be put in the comment section, here i.e. between "---\n" separator and the diffstat, and not in the commit message. > gitweb/README | 7 ++++++- > gitweb/gitweb.perl | 45 +++++++++++++++++++++++++++++++++++++++++---- > 2 files changed, 47 insertions(+), 5 deletions(-) Other that this minor nick (which I guess could be fixed by Junio when applying), this one looks good. -- Jakub Narebski Poland ^ permalink raw reply [flat|nested] 3+ messages in thread
[parent not found: <1263374828-15342-3-git-send-email-warthog9@eaglescrag.net>]
* Re: [PATCH 2/7] gitweb: Add option to force version match [not found] ` <1263374828-15342-3-git-send-email-warthog9@eaglescrag.net> @ 2010-01-13 17:33 ` Jakub Narebski 0 siblings, 0 replies; 3+ messages in thread From: Jakub Narebski @ 2010-01-13 17:33 UTC (permalink / raw) To: John 'Warthog9' Hawley, git; +Cc: warthog9 On Wed, 13 Jan 2010, John 'Warthog9' Hawley wrote: > From: John 'Warthog9' Hawley <warthog9@kernel.org> > > This adds $git_versions_must_match variable, which is set to true s/is set to true value/if set to true,/ > value checks that we are running on the same version of git that we > shipped with, and if not throw '500 Internal Server Error' error. > What is checked is the version of gitweb (embedded in building > gitweb.cgi), against version of runtime git binary used. > > Gitweb can usually run with a mismatched git install. This is more > here to give an obvious warning as to whats going on vs. silently > failing. > > By default this feature is turned off. [moved version info below, after "---" separator] > Signed-off-by: John 'Warthog9' Hawley <warthog9@kernel.org> > Signed-off-by: Jakub Narebski <jnareb@gmail.com> > --- Moved to the correct place: information about patch versioning should be put in comment section[1], not in commit message. [1] Which means between "---" separator and diffstat. > v3 - warthog9: adjust to use die_error instead of recreating it > v2 - Jakub: Changes to make non-default, and change naming > v1 - warthog9: Initial > gitweb/README | 3 +++ > gitweb/gitweb.perl | 28 ++++++++++++++++++++++++++++ > 2 files changed, 31 insertions(+), 0 deletions(-) > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl > @@ -587,6 +590,31 @@ if (defined $maxload && get_loadavg() > $maxload) { > our $git_version = qx("$GIT" --version) =~ m/git version (.*)$/ ? $1 : "unknown"; > $number_of_git_cmds++; > > +# Throw an error if git versions does not match, if $git_versions_must_match is true. > +if ($git_versions_must_match && > + $git_version ne $version) { > + my $admin_contact = > + defined $ENV{'SERVER_ADMIN'} ? ", $ENV{'SERVER_ADMIN'}," : ''; > + my $err_msg = <<EOT; > +Internal Server Error > +<br /> > +</div> > +<hr /> > +<div class="readme"> > +<h1 align="center">*** Warning ***</h1> > +<p> > +This version of gitweb was compiled for <b>@{[esc_html($version)]}</b>, > +however git version <b>@{[esc_html($git_version)]}</b> was found on server, > +and administrator requested strict version checking. > +</p> > +<p> > +Please contact the server administrator${admin_contact} to either configure > +gitweb to allow mismatched versions, or update git or gitweb installation. > +</p> > +EOT > + die_error(500, $err_msg); > +} I'm sorry to be this picky, but don't you think that caller should not need to know "intimate" details on how die_error works. In particular please notice that you must have known that die_error wraps its output in <div>... which is not even mentioned in comments. The second argument of die_error() subroutine is meant to be alternative description of error condition: short and one line. For situations such like this one, where we want to describe error in more details, it would be better, I think, to change signature of die_error() to take (optionally) three arguments, and use 'die_error(500, undef, $err_msg);', with $err_msg starting from '<h1>...</h1>'. @@ -3460,6 +3460,7 @@ sub die_error { my $status = shift || 500; my $error = shift || "Internal server error"; + my $extra = shift; my %http_responses = ( 400 => '400 Bad Request', @@ -3475,8 +3476,13 @@ sub die_error { <br /><br /> $status - $error <br /> -</div> EOF + if (defined $extra) { + print "<hr />\n" . + "$extra\n"; + } + print "</div>\n"; + git_footer_html(); exit; } Other that this minor issue it looks good. -- Jakub Narebski Poland ^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 0/7] Gitweb caching v4 @ 2010-01-13 9:34 John 'Warthog9' Hawley 2010-01-13 9:34 ` [PATCH 1/7] gitweb: Load checking John 'Warthog9' Hawley 0 siblings, 1 reply; 3+ messages in thread From: John 'Warthog9' Hawley @ 2010-01-13 9:34 UTC (permalink / raw) To: git Evening everyone, This is the latest incarnation of gitweb w/ caching. This is finally at the point where it should probably start either being considered for inclusion or mainline, or I need to accept that this will never get in and more perminantely fork (as is the case with Fedora where this is going in as gitweb-caching as a parrallel rpm package). That said this brings the base up to mainline (again), it updates a number of elements in the caching engine, and this is a much cleaner break-out of the tree vs. what I am currently developing against. v4: - major re-working of the caching layer to use file handle redirection instead of buffering output - other minor improvements v3: - various minor re-works based on mailing list feedback, this series was not sent to the mailing list. v2: - Better breakout - You can actually disable the cache now - John 'Warthog9' Hawley John 'Warthog9' Hawley (7): gitweb: Load checking gitweb: Add option to force version match gitweb: Makefile improvements gitweb: Optionally add "git" links in project list page gitweb: Convert output to using indirect file handle gitweb: add a get function to compliment print_local_time gitweb: File based caching layer (from git.kernel.org) Makefile | 91 ++--- gitweb/Makefile | 129 +++++++ gitweb/README | 14 +- gitweb/cache.pm | 283 +++++++++++++++ gitweb/gitweb.css | 6 + gitweb/gitweb.perl | 1021 ++++++++++++++++++++++++++++++---------------------- 6 files changed, 1052 insertions(+), 492 deletions(-) create mode 100644 gitweb/Makefile create mode 100644 gitweb/cache.pm ^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 1/7] gitweb: Load checking 2010-01-13 9:34 [PATCH 0/7] Gitweb caching v4 John 'Warthog9' Hawley @ 2010-01-13 9:34 ` John 'Warthog9' Hawley 2010-01-13 9:34 ` [PATCH 2/7] gitweb: Add option to force version match John 'Warthog9' Hawley 0 siblings, 1 reply; 3+ messages in thread From: John 'Warthog9' Hawley @ 2010-01-13 9:34 UTC (permalink / raw) To: git 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) v3 - warthog9: small additional adjustment to indicate where new load checking should be added for future improvements v2 - Jakub: switching to using v1 - warthog9: Initial creation 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"> -- 1.6.5.2 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH 2/7] gitweb: Add option to force version match 2010-01-13 9:34 ` [PATCH 1/7] gitweb: Load checking John 'Warthog9' Hawley @ 2010-01-13 9:34 ` John 'Warthog9' Hawley 0 siblings, 0 replies; 3+ messages in thread From: John 'Warthog9' Hawley @ 2010-01-13 9:34 UTC (permalink / raw) To: git From: John 'Warthog9' Hawley <warthog9@kernel.org> This adds $git_versions_must_match variable, which is set to true value checks that we are running on the same version of git that we shipped with, and if not throw '500 Internal Server Error' error. What is checked is the version of gitweb (embedded in building gitweb.cgi), against version of runtime git binary used. Gitweb can usually run with a mismatched git install. This is more here to give an obvious warning as to whats going on vs. silently failing. By default this feature is turned off. v3 - warthog9: adjust to use die_error instead of recreating it v2 - Jakub: Changes to make non-default, and change naming v1 - warthog9: Initial Signed-off-by: John 'Warthog9' Hawley <warthog9@kernel.org> Signed-off-by: Jakub Narebski <jnareb@gmail.com> --- gitweb/README | 3 +++ gitweb/gitweb.perl | 28 ++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 0 deletions(-) diff --git a/gitweb/README b/gitweb/README index 6c2c8e1..608b0f8 100644 --- a/gitweb/README +++ b/gitweb/README @@ -233,6 +233,9 @@ not include variables usually directly set during build): 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. + * $git_versions_must_match + If set, gitweb fails with 500 Internal Server Error if the version of gitweb + doesn't match version of git binary. The default is false. Projects list file format diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 0a07d3a..f17593f 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -221,6 +221,9 @@ our %avatar_size = ( 'double' => 32 ); +# If it is true, exit if gitweb version and git binary version don't match +our $git_versions_must_match = 0; + # 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. @@ -587,6 +590,31 @@ if (defined $maxload && get_loadavg() > $maxload) { our $git_version = qx("$GIT" --version) =~ m/git version (.*)$/ ? $1 : "unknown"; $number_of_git_cmds++; +# Throw an error if git versions does not match, if $git_versions_must_match is true. +if ($git_versions_must_match && + $git_version ne $version) { + my $admin_contact = + defined $ENV{'SERVER_ADMIN'} ? ", $ENV{'SERVER_ADMIN'}," : ''; + my $err_msg = <<EOT; +Internal Server Error +<br /> +</div> +<hr /> +<div class="readme"> +<h1 align="center">*** Warning ***</h1> +<p> +This version of gitweb was compiled for <b>@{[esc_html($version)]}</b>, +however git version <b>@{[esc_html($git_version)]}</b> was found on server, +and administrator requested strict version checking. +</p> +<p> +Please contact the server administrator${admin_contact} to either configure +gitweb to allow mismatched versions, or update git or gitweb installation. +</p> +EOT + die_error(500, $err_msg); +} + $projects_list ||= $projectroot; # ====================================================================== -- 1.6.5.2 ^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-01-13 17:34 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1263374828-15342-1-git-send-email-warthog9@eaglescrag.net>
[not found] ` <1263374828-15342-2-git-send-email-warthog9@eaglescrag.net>
2010-01-13 17:18 ` [PATCH 1/7] gitweb: Load checking Jakub Narebski
[not found] ` <1263374828-15342-3-git-send-email-warthog9@eaglescrag.net>
2010-01-13 17:33 ` [PATCH 2/7] gitweb: Add option to force version match Jakub Narebski
2010-01-13 9:34 [PATCH 0/7] Gitweb caching v4 John 'Warthog9' Hawley
2010-01-13 9:34 ` [PATCH 1/7] gitweb: Load checking John 'Warthog9' Hawley
2010-01-13 9:34 ` [PATCH 2/7] gitweb: Add option to force version match John 'Warthog9' Hawley
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 NNTP newsgroup(s).