Mailing-List: contact modules-dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: modules-dev@httpd.apache.org
Received-SPF: pass (athena.apache.org: domain of neil.mckee@inmon.com
 designates 209.85.213.173 as permitted sender)
Subject: Re: mod_sflow
Mime-Version: 1.0 (Apple Message framework v1082)
Content-Type: text/plain; charset=us-ascii
From: Neil McKee <neil.mckee@inmon.com>
In-Reply-To: <AANLkTikd6EaFWYLKYLtniPJW9BDVaPEfBSHYgGkFg21G@mail.gmail.com>
Date: Sun, 9 Jan 2011 23:03:51 -0800
Content-Transfer-Encoding: quoted-printable
Message-Id: <78D18C93-A7D7-48B5-B7B7-B95D4527310F@inmon.com>
References: <64258452-9380-4E36-AD69-298CC66DDB43@inmon.com>
 <AANLkTikd6EaFWYLKYLtniPJW9BDVaPEfBSHYgGkFg21G@mail.gmail.com>
To: modules-dev@httpd.apache.org

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=3D14
>=20
> Neil, two points of critique:
>=20
> 1. You are doing way too much in your critical section, including
> potentially blocking actions like logging.
>=20
> 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.