Re: патч для отключения заголовков "Connection: keep-alive" (еще раз)

[Maxim Dounin]

Hello!

On Tue, Dec 10, 2013 at 05:16:45PM +0600, Илья Шипицин wrote:

> 10 декабря 2013 г., 17:13 пользователь Maxim Dounin
> <mdounin at mdounin.ru> написал:
> > Hello!
> >
> > On Tue, Dec 10, 2013 at 09:13:16AM +0600, Илья Шипицин wrote:
> >
> >> Добрый день!
> >>
> >> как-то уже писал на эту тему. дошли руки, выкатил патч в продакшен.
> >> идея в том, что отправка заголовка "Connection: keep-alive" в
> >> большинстве случаев не нужна.
> >>
> >> абонентская база - сотни тысяч пользователей, сотни миллионов запросов
> >> в месяц. думаю, спустя месяц-два можно будет считать, что все
> >> протестировано во всех возможных ситуациях.
> >>
> >>
> >> желающие - приглашаются к тестированию.
> >>
> >> данное поведение "подсмотрено" у IIS, который по любым метрикам
> >> занимает десятки процентов "рынка":
> >> http://news.netcraft.com/archives/2013/12/06/december-2013-web-server-survey.html
> >> - поэтому есть уверенность, что все обойдется без побочных эффектов
> >
> > Илья, я вроде как сделал патч с таким поведением ещё в процессе
> > прошлого обсуждения:
> >
> > http://mailman.nginx.org/pipermail/nginx-ru/2013-May/051059.html
> >
> > Странно видить попытки изобрести велосипед заново вместо того,
> > чтобы заняться тестированием и последующим убеждением, что это
> > надо закоммитить, тем более про "тщательное тестирование" было
> > явно написано.  ;)
> 
> это именно та тема и есть, сейчас дошли руки "тщательно
> протестировать". я запустил в продакшн.

Меня в первую очередь удивило, что вместо патча, нарисованного ещё 
полгода назад, к письму о "потестировать" прилагается совсем 
другой патч, с чуть-чуть другими условиями и стилистическими 
проблемами.  Я бы всё-таки рекомендовал тестировать то, что потом 
будем коммитить.

> ждем пару месяцев (и, если проблем не всплывает, считаем, что все ок) ?

Я, честно говоря, пребывал в надежде, что патч уже полгода 
тестируется. ;)

Но если нет - то, видимо, так и делаем.  Только просьба - взять 
для тестирования патч в том виде, в каком его предполагается 
коммитить, if any:

# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1386675580 -14400
#      Tue Dec 10 15:39:40 2013 +0400
# Node ID 178020375791a638c79cfe1330bddabbf830f0bf
# Parent  58716fd3bd2d63c93b0c04fa121232b7126e724b
Http: Connection header sending change.

Now "Connection: keep-alive" isn't sent if persistent connections are
used by default (in HTTP/1.1) and we don't add Keep-Alive header.

This is an experimental patch, see here for discussion:
http://mailman.nginx.org/pipermail/nginx-ru/2013-May/051052.html

diff --git a/src/http/ngx_http_header_filter_module.c b/src/http/ngx_http_header_filter_module.c
--- a/src/http/ngx_http_header_filter_module.c
+++ b/src/http/ngx_http_header_filter_module.c
@@ -386,7 +386,10 @@ ngx_http_header_filter(ngx_http_request_
         len += sizeof("Connection: upgrade" CRLF) - 1;
 
     } else if (r->keepalive) {
-        len += sizeof("Connection: keep-alive" CRLF) - 1;
+
+        if (r->http_version < NGX_HTTP_VERSION_11 || clcf->keepalive_header) {
+            len += sizeof("Connection: keep-alive" CRLF) - 1;
+        }
 
         /*
          * MSIE and Opera ignore the "Keep-Alive: timeout=<N>" header.
@@ -559,8 +562,11 @@ ngx_http_header_filter(ngx_http_request_
                              sizeof("Connection: upgrade" CRLF) - 1);
 
     } else if (r->keepalive) {
-        b->last = ngx_cpymem(b->last, "Connection: keep-alive" CRLF,
-                             sizeof("Connection: keep-alive" CRLF) - 1);
+
+        if (r->http_version < NGX_HTTP_VERSION_11 || clcf->keepalive_header) {
+            b->last = ngx_cpymem(b->last, "Connection: keep-alive" CRLF,
+                                 sizeof("Connection: keep-alive" CRLF) - 1);
+        }
 
         if (clcf->keepalive_header) {
             b->last = ngx_sprintf(b->last, "Keep-Alive: timeout=%T" CRLF,

-- 
Maxim Dounin
http://nginx.org/