httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From r..@covalent.net
Subject Re: logging APR errors
Date Mon, 17 Jul 2000 19:31:59 GMT
On Mon, 17 Jul 2000, Greg Marr wrote:
> At 11:35 AM 07/17/2000 -0700, rbb@covalent.net wrote:
> >On Mon, 17 Jul 2000, Jeff Trawick wrote:
> > > The ap_pool_t structure is a convenient place to register an error
> > > call-back, [...]
> > >
> > > ap_status_t ap_set_error(void (*apr_error)(undetermined-signature),
> > >                          void *user_data,
> > >                          ap_pool_t *p);
> > >
> > > parm 1: address of error call-back
> > > parm 2: user data to be passed by APR to the call-back
> > > parm 3: the pool
> >
> >This doesn't work.  Even if we have one callback for all of Apache, we 
> >have to set the user data appropriately.  Because we have to set the user 
> >data, this impacts EVERY call to APR.  You can NEVER be sure that the user 
> >data was set correctly before entering before entering an APR function.
> 
> Isn't there one pool per (server_req/request_req), and one (server_req / 
> request_req) per pool?  If so, then there's no problem, because the 
> user_data is unique per pool, and that's where the callback is registered.

There is one pool per request_rec, but we aren't always in the middle of a
request when we want to log things.  There is also one pool per server,
but we don't always have access to that pool when we want to log.  Plus,
other threads have access to the server_rec's pool.  There is also one
pool per thread, but we don't always have access to that pool.  The
problem is, that the pools we always have access to can be shared, and the
pools that aren't shared we don't always have access to.  :-(

> >Right here, you tie APR to Apache.  By just inserting this logic in places 
> >that Apache cares about it, you make APR less useful to other 
> >programs.  That is un-acceptable.  Either all error conditions need to be 
> >treated equally, or we can't do this approach.  What determines when an 
> >error case gets the call to the registered function?
> 
> "in APR functions which invoke multiple syscalls, and ... where our 
> experience shows that it would be useful."  I don't see anything here that 
> says anything about Apache, unless you consider "our" to refer to Apache 
> developers instead of APR developers.  If the APR function can fail in more 
> ways than can be described with a simple integer error code, or there is 
> additional error information that could be provided, then the callback is 
> used.  If the function's failure can be defined by a small range of 
> integers, then that's what is returned, and no extra error-reporting is needed.

Take a closer look at which information is to be logged.  There is no
reason that the information should be restricted to the functions that
use multiple system calls.  Those functions have already been chosen
because the Apache developers are having problems.  The simple fact that
we are talking about only doing the extra logging for some functions
proves that this is being tied to Apache.

> Why can't the callback know which conditions it wants to log and which it 
> doesn't?  If Apache knows when to log the error and when not to, then 
> Apache knows when to log the error and when not to.

Because the callback doesn't have that kind of state information.  Unless,
Apache sends a flag to APR that says "This is where we are in the
processing", the callback will never have the right information.  The
callback just can't have enough information to know what to log and what
not to log.  If it does have that much information, then that is one more
thing that Apache is passing down, or it is going to be a very large
function.

> > > rbb@covalent.net wrote:
> > > >                          ...              Having every APR program
> > > > be able to register an error logging function adds yet another if
> > > > condition to EVERY single APR function.  Many of those functions have
> > > > multiple error paths.  This means that we get multiple if's for every
APR
> > > > function.  People are already complaining that APR isn't performance
> > > > aware, and now we want to slow it down more.
> > >
> > > 1. I'm not in favor of adding logging to every error path.
> >
> >Too bad.  You can't add code in some cases and not others.  That ties you 
> >to Apache.
> 
> It only ties to Apache if the decision whether or not to add code is made 
> with knowledge of what Apache wants.

See above.

> 
> > > 2. This is the error path we're talking about after all.  We can't
> > >    compare the almost-nil performance hit for adding an if and a
> > >    function call to an error path with some of the APR performance
> > >    issues which have been demonstrated recently.
> >
> >Yes, this is true, iff we only make the callback in some places, we don't 
> >add extra function calls to the non-error cases.  Since we can't just make 
> >the callback in some places, we have to add at least one extra call in ALL 
> >cases.
> 
> Regardless of whether or not we only make the callback in some places, we 
> don't add extra function calls to the non-error cases.  No non-error cases 
> call the function, since it's an error-reporting function.

But, since Apache has to decide if we are logging the error or not, we do
need that function call.

There are multiple calls to ap_stat.  We log failures for some of these,
and not for others.  Unless Apache de-registers the callback function, or
pushes a NULL function pointer, we will log ALL failures for
ap_stat.  This need to modify the error handling stack MUST be done for
all APR calls, for reasons outlined in earlier messages.  This means that
we must add a function call in the main-line code.  Unless the "don't
log" flag is passed to the error logging callback.  Which just adds to the
hackiness of this solution.

Ryan

_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


Mime
View raw message