Re: [PATCH 1/6] GITWEB - Load Checking

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



<snip>
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).

Well there's the opposite argument that setting the number arbitrarily high, 4096 for instance would also in essence negate this (though I'll admit I've reached and exceeded those numbers before)

That said I agree, being able to turn this off needs to be added and will be shortly.

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?

Will comment on this further down

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.

That's fine.


Signed-off-by: John 'Warthog9' Hawley <warthog9@xxxxxxxxxxxxxx>

Why signoff is different from author (warthog9@xxxxxxxxxx)?  Why this
email for signoff?  Just curious...

My bad, did the patches up on my laptop but had to send them out from kernel.org, thus the miss-match: I.E. user error.

<snip>

+# 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?

Here's the fundamental problem:

Sys:Info:Device:CPU
	Windows:
		Using this method under Windows is not recommended
		since, the WMI interface will possibly take at least 2
		seconds to complete the request.

BSD::getloadavg
	While this more or less supports anything with a libc getloadavg
	(and thus might be the best one I've seen, I'll admit I didn't
	notice this one when I looked years ago) getting it to work on
	windows looks, exciting.

Sys::CpuLoad:
	http://cpansearch.perl.org/src/CLINTDW/Sys-CpuLoad-0.03/README
	Specifically:
		- Currently FreeBSD and OpenBSD are supported.
		- Wanted: HPUX 11.11 ...
		- Todo: Win32 support

	So this doesn't really buy me anything but, maybe, BSD support.
	
So at the end of the day, none of those really gets me a "useful" cross platform load checker (though like I said BSD::getloadavg looks to be the best of the ones you mentioned) and more or less Windows is going to lose this as a usable feature no matter what.

I think I'd almost rather set this up so that if it can't get something useful (I.E. /proc/loadavg is missing) it just skips past it as if the load was 0.

I might try out the BSD::getloadavg but I want to take a look and see if that's easily installed or not, if it's not it might be difficult to justify that as a dependency.

<snip>

+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;

It was intended to be the most minimal possible, mainly get in, get out. Also not sure the die_error existed in gitweb when this was originally written. Probably worth switching to it now since it's there either way, and I don't think using it would add enough overhead to matter.

- John 'Warthog9' Hawley
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]