Re: cvs commit: httpd-2.0/server protocol.c

[Ben Laurie <be...@algroup.co.uk>]

gregames@apache.org wrote:
> 
> gregames    01/03/20 18:20:05
> 
>   Modified:    server   protocol.c
>   Log:
>   back out the logging of read errors in getline.  daedalus was logging
>   boatloads of "(54)Connection reset by peer: ap_get_brigade() failed" errors.

Wouldn't it be better to filter out connection reset by peer - now we
get no diagnostic at all if something weird happens?

Cheers,

Ben.

--
http://www.apache-ssl.org/ben.html

"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff

ApacheCon 2001! http://ApacheCon.com/

Re: cvs commit: httpd-2.0/server protocol.c

[Greg Ames <gr...@remulak.net>]

Ben Laurie wrote:
> 
> gregames@apache.org wrote:
> >
> > gregames    01/03/20 18:20:05
> >
> >   Modified:    server   protocol.c
> >   Log:
> >   back out the logging of read errors in getline.  daedalus was logging
> >   boatloads of "(54)Connection reset by peer: ap_get_brigade() failed" errors.
> 
> Wouldn't it be better to filter out connection reset by peer - now we
> get no diagnostic at all if something weird happens?
> 

I thought about that briefly last night.  My main objective was to
quickly get daedalus up and running on a stable 2.0 code base, then make
the code available to anyone else who might want to do the same.  btw,
no core dumps yet on this build (knock on wood).

I'd rather not have a piece of code in ap_getline that evolves into
something like the accept() error handling code in the mpm's.  It's
fairly long, complex, infrequently executed, and physically right in the
middle of mainline path code.  That clutters up the CPU's instruction
cache with mostly dead instructions.  ap_getline is even more
performance sensitive than child_main, since it is called for every
header line as well as the request line.

Yes, we could move the complex error legs into separate functions and
avoid the i-cache hits.  But in this case, I'm wondering how helpful
these log entries would be?  If it's the read of the request line, there
are probably lots of common reasons why it could fail that we don't care
about.  Except for a few hours yesterday, we treated it as if the
request never existed, and no one complained.  I would be using tcpdump
or something similar to debug initial read failures.  I believe 1.3
silently ignores read errors.  If the connection dies, most often you
see write errors logged.  

Reads of mime headers might be more interesting.  That's what was
causing our assert failures.  We were getting "connection reset by peer"
failures very infrequently while the mime headers were being read.  But
since it was core dumping, we were looking for clues in the dumps, not
in the logs.  It might be better if APR saved the last bad apr_status in
the apr_socket.  The code would be simple, it would be very helpful for
debugging a core dump related to network errors, but it wouldn't clutter
up the logs.

Greg