You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by "Roy T. Fielding" <fi...@kiwi.ics.uci.edu> on 1998/02/09 01:00:05 UTC

Re: cvs commit: apache-1.3/src/main http_main.c http_protocol.c

ARRRGGGGHHH!!   I vetoed this patch on the list.  It should never
have been applied.  It is dumb, DUMB, DUMB!!!  Aside from being just
a bad idea and "fixing" a problem that was already fixed in 1.3b,
APLOG_NOTICE is a higher logging level than APLOG_WARNING in Apache.
That means the log message will *always* be printed with this patch.

....Roy

In message <19...@hyperreal.org>, brian@hyperreal.org writ
es:
>brian       98/02/06 18:29:20
>
>  Modified:    src/main http_main.c http_protocol.c
>  Log:
>  Consensus on this list seemed to be that the "send lost connection"
>  messages, while not a "debug"-level message, was also not a "warning"
>  message, as frequently there's not much that a server administrator
>  can do to mitigate those reports directly.  Swarms of lost connections
>  may indicate connection troubles, but individual ones are not worth
>  filling logfiles with.
>  
>  Revision  Changes    Path
>  1.282     +1 -1      apache-1.3/src/main/http_main.c
>  
>  Index: http_main.c
>  ===================================================================
>  RCS file: /export/home/cvs/apache-1.3/src/main/http_main.c,v
>  retrieving revision 1.281
>  retrieving revision 1.282
>  diff -u -r1.281 -r1.282
>  --- http_main.c	1998/02/06 17:59:29	1.281
>  +++ http_main.c	1998/02/07 02:29:17	1.282
>  @@ -841,7 +841,7 @@
>       }
>   
>       if (!current_conn->keptalive)
>  -	aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_WARNING,
>  +	aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_NOTICE,
>   		    current_conn->server, errstr);
>   
>       if (timeout_req) {
>  
>  
>  
>  1.186     +3 -3      apache-1.3/src/main/http_protocol.c
>  
>  Index: http_protocol.c
>  ===================================================================
>  RCS file: /export/home/cvs/apache-1.3/src/main/http_protocol.c,v
>  retrieving revision 1.185
>  retrieving revision 1.186
>  diff -u -r1.185 -r1.186
>  --- http_protocol.c	1998/02/04 21:23:33	1.185
>  +++ http_protocol.c	1998/02/07 02:29:18	1.186
>  @@ -1688,7 +1688,7 @@
>                   else if (errno == EAGAIN)
>                       continue;
>                   else {
>  -                    aplog_error(APLOG_MARK, APLOG_WARNING, r->server,
>  +                    aplog_error(APLOG_MARK, APLOG_NOTICE, r->server,
>                                   "send body lost connection to %s",
>                                   get_remote_host(r->connection,
>                                                   r->per_dir_config,
>  @@ -1777,7 +1777,7 @@
>                   else if (errno == EAGAIN)
>                       continue;
>                   else {
>  -                    aplog_error(APLOG_MARK, APLOG_WARNING, r->server,
>  +                    aplog_error(APLOG_MARK, APLOG_NOTICE, r->server,
>                                   "send body lost connection to %s",
>                                   get_remote_host(r->connection,
>                                                   r->per_dir_config,
>  @@ -1845,7 +1845,7 @@
>                   else if (errno == EAGAIN)
>                       continue;
>                   else {
>  -                    aplog_error(APLOG_MARK, APLOG_WARNING, r->server,
>  +                    aplog_error(APLOG_MARK, APLOG_NOTICE, r->server,
>                                   "send mmap lost connection to %s",
>                                   get_remote_host(r->connection,
>                                                   r->per_dir_config,
>  
>  
>  


Re: cvs commit: apache-1.3/src/main http_main.c http_protocol.c

Posted by Dean Gaudet <dg...@arctic.org>.
On Sun, 8 Feb 1998, Roy T. Fielding wrote:

> ARRRGGGGHHH!!   I vetoed this patch on the list.  It should never
> have been applied.  It is dumb, DUMB, DUMB!!!  Aside from being just
> a bad idea and "fixing" a problem that was already fixed in 1.3b,
> APLOG_NOTICE is a higher logging level than APLOG_WARNING in Apache.
> That means the log message will *always* be printed with this patch.
> 
> ....Roy
> 
> In message <19...@hyperreal.org>, brian@hyperreal.org writ
> es:
> >brian       98/02/06 18:29:20
> >
> >  Modified:    src/main http_main.c http_protocol.c
> >  Log:
> >  Consensus on this list seemed to be that the "send lost connection"
> >  messages, while not a "debug"-level message, was also not a "warning"
> >  message, as frequently there's not much that a server administrator
> >  can do to mitigate those reports directly.  Swarms of lost connections
> >  may indicate connection troubles, but individual ones are not worth
> >  filling logfiles with.

And I still completely disagree with both you and Chuck, who are the only
two complaining about this... and neither of you have to take the time to
tell users "no ignore that stupid warning" when they open PRs about it. 

This warning/notice/whatever is useless.  It diagnoses a normal situation. 

I can cause this warning to litter my logs by dialing in at 28.8 and
clicking on links before the page completely loads.  This is how users
normally surf.

Timeouts I can see being reported.  But not the normal "connection closed
prematurely" stuff.  In other words, I think the following patch should
be applied.

Dean

Index: main/http_main.c
===================================================================
RCS file: /export/home/cvs/apache-1.3/src/main/http_main.c,v
retrieving revision 1.284
diff -c -r1.284 http_main.c
*** http_main.c	1998/02/08 18:16:01	1.284
--- http_main.c	1998/02/09 00:19:49
***************
*** 813,819 ****
  
  void timeout(int sig)
  {				/* Also called on SIGPIPE */
-     char errstr[MAX_STRING_LEN];
      void *dirconf;
  
      signal(SIGPIPE, SIG_IGN);	/* Block SIGPIPE */
--- 813,818 ----
***************
*** 833,852 ****
  	dirconf = timeout_req->per_dir_config;
      else
  	dirconf = current_conn->server->lookup_defaults;
!     if (sig == SIGPIPE) {
! 	ap_snprintf(errstr, sizeof(errstr), "%s lost connection to %s",
  		    timeout_name ? timeout_name : "request",
  		    get_remote_host(current_conn, dirconf, REMOTE_NAME));
      }
-     else {
- 	ap_snprintf(errstr, sizeof(errstr), "%s timed out for %s",
- 		    timeout_name ? timeout_name : "request",
- 		    get_remote_host(current_conn, dirconf, REMOTE_NAME));
-     }
- 
-     if (!current_conn->keptalive)
- 	aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_NOTICE,
- 		    current_conn->server, errstr);
  
      if (timeout_req) {
  	/* Someone has asked for this transaction to just be aborted
--- 832,850 ----
  	dirconf = timeout_req->per_dir_config;
      else
  	dirconf = current_conn->server->lookup_defaults;
!     /* SIGPIPE is generated when the user closes the connection
!      * while browsing... which is a normal situation, and is not
!      * worth reporting.  But timeouts are worth reporting because
!      * they can indicate that the Timeout is set poorly for the
!      * site's users.
!      */
!     if (sig != SIGPIPE && !current_conn->keptalive) {
! 	aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_WARNING,
! 		    current_conn->server,
! 		    "%s timed out for %s",
  		    timeout_name ? timeout_name : "request",
  		    get_remote_host(current_conn, dirconf, REMOTE_NAME));
      }
  
      if (timeout_req) {
  	/* Someone has asked for this transaction to just be aborted