Nginx-ru mailing list archive (nginx-ru@sysoev.ru)
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: патч для Connection: Keep-Alive
Hello!
On Tue, May 14, 2013 at 09:39:42PM +0600, Илья Шипицин wrote:
> 14 мая 2013 г., 21:16 пользователь Maxim Dounin <mdounin@xxxxxxxxxx> написал:
> > 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@xxxxxxxxx
http://mailman.nginx.org/mailman/listinfo/nginx-ru
|