Hi,
Alvaro Lopez Ortega wrote:
Good stuff!! The patch looks really good :-)
Thanks! ;)
> Feel free to give any suggestions, ideas, criticism, etc.
I have only one comment..
> +ret_t
> +cherokee_throttler_wait (void)
> +{
[..]
> + ts.tv_sec = 0;
> + ts.tv_nsec = nsec;
> + nanosleep (&ts, NULL);
> +}
We should not even think of sleep inside a thread of the server.. it
is generating a potential DOS problem and will decrease the server
performance.
Have you think of returning a ret_eagain? It is not neither the
perfect solution (because it will more CPU hungry), but I think it
is fair enough by the moment.
I've already tried returning ret_eagain, but it'll consume 100% CPU even
with a single connection on the server.
This happens when the thread is on 'process_active_connections' and
there's not enought bandwidth to proceed. The thread will keep looping
throught connections and finally return ret_eagain back to
'cherokee_thread_step_MULTI_THREAD', which in turn will ignore that
return value and be called again on 'thread_routine' to process the next
step, and so on.
This will happen many many times if there's no bandwidth for a short
period of time, which is very common, and consume 100% CPU.
The solution for this problem on the patch is:
1 - Calculate the minimum sleep time based on the clock resolution for
the current implementation. On recent Linux kernels it's 10ms, or
10000000ns.
2 - If there's no bandwidth for at least one connection, at the end of
the loop that thread must sleep, otherwise keep going.
I completely agree that this is not the best approach.
Do you have any ideas?
Thanks.
Diego Giagio
_______________________________________________
Cherokee mailing list
[email protected]
http://www.alobbs.com/cgi-bin/mailman/listinfo/cherokee
