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 20:47:31 GMT

> > This isn't the design being discussed.  The design being discussed has us
> > adding code to APR for those cases that Apache wants more
> > information.  This is not a good thing.  We would need to add code to APR
> > for all cases.  Then, Apache needs to decide whether or not it wants to
> > log an error before EVERY APR call.  There is no way around that.
> 
> One macro, expanding to two or three lines of code, and we are done.  Now,
> your speed concern... the apr_pool_t structure with the stack reference, we
> are dereferencing twice to determine noone needs the error by testing for
> null.  Sounds like a candidate for the CPU caching it all, with very little
> impact on performance.  When we want to log it, we get the finest grained
> detail we could ask for.  When we don't, we move quickly.

Yes, this can be fast.  It still doesn't solve all of the problems
however.  If we create two logging function, one for log_error and one for
log_rerror plus NULL for the case where we don't want to log), we have to
push and pop those to the pool whenever we enter and leave an APR
function.  This requires two lines of code, macros, that need to be added
before and after EACH APR call.

One of the other arguments for this approach, is that this can be done
with a minimal amount of change to the server.  This is just not true.

It still doesn't solve the magically logging in some cases, and making the
app log in other cases.

> > If Apache doesn't want to log an error, it has to look at the stack, and
> > push NULL, or leave the stack alone.  If Apache does want to log an error
> > it has to either look at the stack and add log_error_callback or leave the
> > stack alone.
> 
> Hold on - if Apache doesn't want to log an error - right there, we don't know
> that the parent doesn't want us to log or not log the error.  Rule is, if you
> push null to the error condition, you better be prepared not to care.  Most
> blocks of code -do- care that they failed.  I see three situations:

That's fine, but that also means that either Apache or APR has to pop that
NULL off the stack on returning from the function.  You are about to
wrapper EVERY APR call with a push and a pop.

>   1) A module calls ap_stat to find if a file exists.  It won't care that
>      there is no file.  It pushes NULL.  (Now, what do we do with the error
>      if it -wasn't- ENOFILE???  This really requires more thought)
>      
>   2) A module calls ap_stat on a file it believes exists.  If it fails the
>      whole call will bail.  Here we do nothing.  Why?  Because the only call
>      that cared that we bailed was the caller of our own function.  By definiton,
>      the apr_pool_t had a null at the head of the stack, so we can ignore the
>      error handling all together, and return the status to indicate failure.

You are changing the design from what Jeff has proposed.  Just so long as
we are aware of that.

>   3) A module calls ap_stat, but we want to report something other than the
>      standard not-found message.  We push a handler.  But this handler must
>      be smart enough to pass up the error to an acutally logger rather than a
>      handler.  We don't know/care where or what will be logged, we are just
>      making a pretty message (with the filename that couldn't be stat'ted).
>      We could even look ahead for a null blocking point to know that there
>      is noone even -reading- our message :)  If an error string falls in the
>      heap and noone's listening, it still wastes cycles/space.

But what does the handler actually do?  In the case of Apache, it must
call either ap_log_error or ap_log_rerror.  In either case, it needs to
have either a server_rec or a request_rec to be able to do that with any
kind of information.  In the current design, that information is stored in
the pool, but the pools aren't thread safe, so that doesn't work.

Basically, this opens up a big can of worms that really isn't
needed.  Just return a more detailed error value.

> > If we just use a stack, and just push a function when we want to log, and
> > pop after the APR call, then we are still impacting the main-line
> > case.  The advantage to this approach, was that it doesn't impact the
> > main-line case.  This is untrue.  There is no way for a logging callback
> > function to not impact the main-line case that I can see.
> 
> The mainline case says we don't know and don't care to report the error.  We
> didn't hook it.  There are two exceptions, we care to create our own message
> or we don't want anyone to see the error.  Both are the non-mail-line cases.

No.  The mainline case doesn't have anything to do with whether or not we
care about the error message.  The mainline case is that there is no error
message to care about.  Anytime we have an error message, we can let the
performance slip a bit.  The problem, is that every design discussed so
far causes a performance hit (even if it's just a small one) when there is
no error to report.  Either we push and pop anytime we want to report a
detailed error, or we pop and push whenever we don't want to report an
error.

> I know this is far from thought out, never mind designed.  But if we have a
> message, we are prepared to wire the message.  If we don't, we let the
> functions we call and the function that called us work all that out.

The biggest problem I have with this, is that now the app is logging the
error sometimes, and APR is calling back into the app to log it other
times.  This is bad.  Plus, we don't have just one function that we can
push to the stack, we have at least two, plus a NULL for no error.

What is wrong with just adding a couple of more error values?  They don't
give an exact line/file that the problem occurred on.  So what.  They give
enough information that it is possible to find the function, and within
that function the system call that failed.

They cause NO overhead in any case.  They require only the code to be
changed by adding mroe error values.  If we want, we can even do something
like:

ap_sendfile(...)
{
setsocketopt(...)
	return (AP_SENDFILE_SOCK_OPT_ERROR+errno)
...
}

This will give us as much information as we could possibly want.

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


Mime
View raw message