Jakub Narebski wrote:
> On Thu, 23 Dec 2010, Jonathan Nieder wrote:
>> This seems to remove the last user of the DONE_GITWEB label. Why not
>> delete the label, too?
>
> Well, actually this patch is in this series only for the label ;-)
>
> Anyway, I can simply drop this patch, and have next one in series
> (adding exception-based error handling, making die_error work like
> 'die') delete DONE_GITWEB label...
I like the current order (first the brief patch to change the
semantics, then the more ambitious change to an eval {} based error
handling implementation), but it doesn't matter so much.
>> die_error gets called when server load is too high; I wonder whether
>> it is right to go back for another request in that case.
>
> If client (web browser) are requesting connection, we have to tell it
> something anyway.
Right, I should have thought a few seconds more. Respawning
gitweb.perl would generate _more_ load[1].
>> A broken per-request (or other) configuration could potentially leave
>> a gitweb process in a broken state,
[...]
> 'die $@ if $@' would call CORE::die, which means it would end gitweb
> process.
This is referring to a later patch?
> For CGI server it doesn't matter anyway, as for each request the process
> is respawned anyway (together with respawning Perl interpreter), and I
> think that ModPerl::Registry and FastCGI servers monitor process that it
> is to serve requests, and respawn it if/when it dies.
Sorry, that was unclear of me. I meant that buggy configuration could
leave a gitweb process in buggy but alive state and frequent failing
requests might be a way to notice that. Contrived example (just to
illustrate what I mean):
our $version .= ".custom";
if (length $version >= 1000) { # untested, buggy code goes here.
@diff_opts = ("--nonsense");
}
I think I was not right to worry about this, either. It is better to
make such unusual and buggy configurations as noticeable as possible
so they can be fixed.
[...]
> But actually handle_errors_html gets called only from fatalsToBrowser,
> which in turn gets called from CGI::Carp::die... which ends calling
> CODE::die (aka realdie), which ends CGI process anyway.
>
> That is why die_error ends with
>
> goto DONE_GITWEB
> unless ($opts{'-error_handler'});
>
> i.e. it doesn't goto DONE_GITWEB nor DONE_REQUEST if called from
> handle_errors_html anyway.
[...]
> Thanks a lot for your comments.
Thanks for a thorough explanation. For what it's worth, with or
without removal of the DONE_GITWEB: label,
Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
[1] I can imagine scenarios in which exiting gitweb would help
alleviate the load, involving:
- large memory footprint for each gitweb process forcing the system
into swapping (e.g., from a memory leak), or
- FastCGI-like server noticing the load and choosing to decrease the
number of gitweb instances.
In the usual case, presumably gitweb memory footprint is small and
FastCGI-like servers limit the number of gitweb instances to a modest
fixed number.
--
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