Re: патч для Connection: Keep-Alive
Maxim Dounin
mdounin at mdounin.ru
Tue May 14 16:37:58 UTC 2013
Hello!
On Tue, May 14, 2013 at 10:24:31PM +0600, Илья Шипицин wrote:
> не слать "Connection: Keep-Alive" для HTTP/1.1 - статистически то же
> самое, что я предлагал (ввиду малораспространенности
> не-кипэлайв-соединений)
>
> давайте так и сделаем ?
Я очень не хочу это трогать, по крайней мере - без тщательного
тестирования.
> вариант "не слать Connection никогда" мы на кошках проверим, потом расскажу.
В рассылках периодически жалуются даже на те очень нечастые
ситуцаии, когда nginx так всё-таки делает (e.g., при
переконфигурации).
>
> 14 мая 2013 г., 22:17 пользователь Maxim Dounin <mdounin at mdounin.ru> написал:
> > Hello!
> >
> > On Tue, May 14, 2013 at 09:39:42PM +0600, Илья Шипицин wrote:
> >
> >> 14 мая 2013 г., 21:16 пользователь Maxim Dounin <mdounin at mdounin.ru> написал:
> >> > Hello!
> >> >
> >> > On Tue, May 14, 2013 at 06:15:05PM +0600, Илья Шипицин wrote:
> >> >
> >> > [...]
> >> >
> >> >> во всех случаях, когда добавлялся Keep-Alive: timeout, добавлялся
> >> >> также и Connection. не вижу ничего некорректного. можете уточнить ?
> >> >>
> >> >> другое дело, что я накосячил и не добавлял Keep-Alive: timeout там,
> >> >> где предполагалось. да, признаю ошибку. вложил новый патч.
> >> >
> >> > Это я не досмотрел патч, и предположил не ту ошибку из двух
> >> > возможных.
> >> >
> >> > [...]
> >> >
> >> >> --- src/http/ngx_http_header_filter_module.c.orig Tue May 14 12:17:59 2013
> >> >> +++ src/http/ngx_http_header_filter_module.c Tue May 14 16:04:29 2013
> >> >> @@ -382,7 +382,7 @@
> >> >> if (r->headers_out.status == NGX_HTTP_SWITCHING_PROTOCOLS) {
> >> >> len += sizeof("Connection: upgrade" CRLF) - 1;
> >> >>
> >> >> - } else if (r->keepalive) {
> >> >> + } else if ((r->keepalive) && ((r->http_version == NGX_HTTP_VERSION_10) || (clcf->keepalive_header)) ) {
> >> >> len += sizeof("Connection: keep-alive" CRLF) - 1;
> >> >>
> >> >> /*
> >> >> @@ -397,9 +397,7 @@
> >> >> len += sizeof("Keep-Alive: timeout=") - 1 + NGX_TIME_T_LEN + 2;
> >> >> }
> >> >>
> >> >> - } else {
> >> >> - len += sizeof("Connection: close" CRLF) - 1;
> >> >> - }
> >> >> + }
> >> >
> >> > "Connection: close" при выключенном keepalive'е нужно указывать,
> >> > не возвращать соответствующий заголовок - чревато ненужными
> >> > проблемами и безуспешными попытками клиентов послать в то же
> >> > соединение следующий запрос.
> >> >
> >>
> >> я понимаю ваши опасения, однако с точки зрения буквы закона, rfc 2616
> >>
> >> 8.1.4 Practical Considerations
> >>
> >>
> >> ........
> >>
> >> A client, server, or proxy MAY close the transport connection at any
> >> time.
> >> .........
> >>
> >>
> >>
> >
> > Это _можно_ делать, но на практике - это приводит к сильно
> > неоптимальному поведению клиентов. Поэтому убирать "Connection:
> > close" из ответов, keepalive в которых не предполагается - это
> > плохая идея.
> >
> > С учётом того, что keepalive в нормальных условиях влючён чуть
> > более, чем всегда - убирать "Connection: close" не представляется
> > разумным ни с какой точки зрения.
> >
> >> с точки зрения духа закона так поступает IIS (по данным Netcraft-а 16%
> >> сайтов), как-то же это работает ...
> >
> > Специально сходил проверил на первый попавшийся сайт с IIS'ом -
> > _так_ IIS, насколько я вижу, не поступает:
> >
> > $ telnet W3schools.com 80
> > Trying 66.29.212.73...
> > Connected to W3schools.com.
> > Escape character is '^]'.
> > HEAD / HTTP/1.1
> > Host: w3scools.com
> >
> > HTTP/1.1 200 OK
> > Cache-Control: private
> > Content-Length: 29839
> > Content-Type: text/html
> > Server: Microsoft-IIS/7.5
> > Set-Cookie: ASPSESSIONIDCABDDTCD=OMAGHFBBMDCIEKCKOIEGKPAO; path=/
> > X-Powered-By: ASP.NET
> > Date: Tue, 14 May 2013 16:10:08 GMT
> >
> > HEAD / HTTP/1.1
> > Host: w3scools.com
> > Connection: close
> >
> > HTTP/1.1 200 OK
> > Cache-Control: private
> > Content-Length: 29839
> > Content-Type: text/html
> > Server: Microsoft-IIS/7.5
> > Set-Cookie: ASPSESSIONIDCABDDTCD=MICGHFBBKNHMJHAFKDAJJOJD; path=/
> > X-Powered-By: ASP.NET
> > Date: Tue, 14 May 2013 16:10:28 GMT
> > Connection: close
> >
> > Connection closed by foreign host.
> >
> > Т.е. если соединение закрывается - то "Connection: close" в ответе
> > присутствует.
> >
> >> у нас 50 миллионов запросов в день. это в районе 1 гигабайта на
> >> хедерах "Connection:". нам эта цифра интересна.
> >> мы, пожалуй, потестируем, всех желающих тоже приглашаю к тесту.
> >>
> >>
> >> > Вообще я бы, честно говоря, не трогал это место.
> >>
> >> это абстрактные опасения или вы знаете сценарии, при которых это
> >> приведет к проблемам ?
> >
> > При правильной реализации - т.е. не слать "Connection: keep-alive"
> > для соединений по HTTP/1.1, если заголовок Keep-Alive не отдаётся -
> > только абстрактные. Все изложенные выше замечания - вполне
> > конкретные.
> >
> > Just in case, правильный патч какой-то такой:
> >
> > --- a/src/http/ngx_http_header_filter_module.c
> > +++ b/src/http/ngx_http_header_filter_module.c
> > @@ -383,7 +383,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.
> > @@ -556,8 +559,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/en/donation.html
> >
> > _______________________________________________
> > nginx-ru mailing list
> > nginx-ru at nginx.org
> > http://mailman.nginx.org/mailman/listinfo/nginx-ru
> _______________________________________________
> nginx-ru mailing list
> nginx-ru at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-ru
--
Maxim Dounin
http://nginx.org/en/donation.html
Подробная информация о списке рассылки nginx-ru