httpd-modules-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Neil McKee <neil.mc...@inmon.com>
Subject Re: mod_sflow
Date Mon, 10 Jan 2011 21:22:32 GMT
I pushed changes to:

1.  use PIPE_BUF from limits.h (if available).
2.  use apr_file_pipe_timeout_set(<pipe>, 0) on both ends -- just to make absolutely
certain that the writes from the critical section are always non-blocking.
3.  use apr_poll() at the read end of the pipe to get my read timeout back.
4.  make sure any critical-section logging is with APLOG_DEBUG.

http://code.google.com/p/mod-sflow/source/browse/trunk/mod_sflow.c?r=17

Neil

On Jan 9, 2011, at 11:03 PM, Neil McKee wrote:

> Great feedback,  thanks.
> 
> I have several questions,   if you don't mind...
> 
> 1. What is the most portable way to make the pipe non-blocking?  That way we can just
drop any samples that get EWOULDBLOCK or EAGAIN on writing to the pipe (and increment the
drops counter).    apr_file_pipe_create_ex(.... APR_FULL_NONBLOCK...) was my first choice,
 but it only seems to have appeared fairly recently,  in APR version 1.3.
> 
> 2.  It's very unlikely that we would ever be sending a message on the pipe greater than
about 200 bytes,  so if we ever see (msgBytes > PIPE_BUF) then I'm OK with just dropping
that message.   PIPE_BUF seems to be defined without explicitly adding "limits.h" as another
include.   Can I assume it will always be defined,  or is there something like "APR_PIPE_BUF"
I should use instead?
> 
> 3.  Making sure we can't block in the critical section is obviously the most important
thing,  but let me know if you still think we need to refine it further after that.  It is
my assumption that the sampling-rate setting will be at least 1-in-100 (and quite possibly
1-in-50000) so the critical path is really just a few counter increments and a sampling decrement-and-test.
   If anyone ever wanted to run it flat out at 1-in-10 or 1-in-1 then obviously we'd have
to look closer at this,  but there are other tools for that kind of logging.
> 
> The sFlow standard allows for multiple independent "sub-agents" to maintain their own
sequence numbers and send their own datagrams (an important property for a large network switch),
 so theoretically we could do without the pipe and have each child process send his own sFlow.
We could even have a separate sub-agent for each worker thread, and run with no locks at all.
 However we decided against that because it could mean generating an uncontrolled number of
these sub-agents,  which would be antisocial for the collector.
> 
> Atomic operations might help to avoid thread-locking here,  but I was hoping to avoid
the "dark arts".
> 
> 4.  The pipe is anonymous.  The docs say that some platforms don't allow that and a filename
is required.  Do I need to bother with that,  or are those platforms obsolete anyway?
> 
> I apologize if any of these are FAQs / RTFMs.  I'm new to APR.
> 
> Neil
> 
> 
> On Jan 9, 2011, at 12:17 PM, Ben Noordhuis wrote:
> 
>> On Sat, Jan 8, 2011 at 00:06, Neil McKee <neil.mckee@inmon.com> wrote:
>>> This module is designed to work in both "prefork" and "worker" models.  I would
really
>>> appreciate it if someone could review the design to make sure I made appropriate
choices
>>> about where to use pipes, shared-memory, mutex locking,  and so on(!)   These
choices
>>> are documented in the comment at the top of the mod_sflow.c file,  here:
>>> http://code.google.com/p/mod-sflow/source/browse/trunk/mod_sflow.c?r=14
>> 
>> Neil, two points of critique:
>> 
>> 1. You are doing way too much in your critical section, including
>> potentially blocking actions like logging.
>> 
>> 2. Assuming it's safe to write up to 4K to the pipe is dangerous for
>> several reasons: PIPE_BUF may be < 4096, the pipe may not be empty,
>> etc. This ties in with #1 since you are doing it from within the
>> critical section.
> 


Mime
View raw message