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

[Илья Шипицин <chipitsine@xxxxxxxxx>]

10 декабря 2013 г., 19:37 пользователь Maxim Dounin
<mdounin@xxxxxxxxxx> написал:
> Hello!
>
> On Tue, Dec 10, 2013 at 05:16:45PM +0600, Илья Шипицин wrote:
>
>> 10 декабря 2013 г., 17:13 пользователь Maxim Dounin
>> <mdounin@xxxxxxxxxx> написал:
>> > 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:

без проблем. сегодня запущу в продакшн в таком виде

не совсем понятно, зачем делать условие "r->http_version < NGX_HTTP_VERSION_11"
для версии 0.9, насколько я понимаю, и в моем и в вашем случае будет фигня


>
> # HG changeset patch
> # User Maxim Dounin <mdounin@xxxxxxxxxx>
> # 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/
>
> _______________________________________________
> nginx-ru mailing list
> nginx-ru@xxxxxxxxx
> http://mailman.nginx.org/mailman/listinfo/nginx-ru
_______________________________________________
nginx-ru mailing list
nginx-ru@xxxxxxxxx
http://mailman.nginx.org/mailman/listinfo/nginx-ru