[nginx] SSL: lowered log level for WSAECONNABORTED errors on Windows.

2019-08-16 Thread Maxim Dounin
details:   https://hg.nginx.org/nginx/rev/2432a687e789
branches:  
changeset: 7560:2432a687e789
user:  Maxim Dounin 
date:  Fri Aug 16 18:16:21 2019 +0300
description:
SSL: lowered log level for WSAECONNABORTED errors on Windows.

Winsock uses ECONNABORTED instead of ECONNRESET in some cases.
For non-SSL connections this is already handled since baad3036086e.

Reported at
http://mailman.nginx.org/pipermail/nginx-ru/2019-August/062363.html.

diffstat:

 src/event/ngx_event_openssl.c |  3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diffs (13 lines):

diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c
--- a/src/event/ngx_event_openssl.c
+++ b/src/event/ngx_event_openssl.c
@@ -2814,6 +2814,9 @@ ngx_ssl_connection_error(ngx_connection_
 if (sslerr == SSL_ERROR_SYSCALL) {
 
 if (err == NGX_ECONNRESET
+#if (NGX_WIN32)
+|| err == NGX_ECONNABORTED
+#endif
 || err == NGX_EPIPE
 || err == NGX_ENOTCONN
 || err == NGX_ETIMEDOUT
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel


[nginx] Version bump.

2019-08-16 Thread Maxim Dounin
details:   https://hg.nginx.org/nginx/rev/26281dcecfbc
branches:  
changeset: 7559:26281dcecfbc
user:  Maxim Dounin 
date:  Fri Aug 16 18:16:14 2019 +0300
description:
Version bump.

diffstat:

 src/core/nginx.h |  4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diffs (14 lines):

diff --git a/src/core/nginx.h b/src/core/nginx.h
--- a/src/core/nginx.h
+++ b/src/core/nginx.h
@@ -9,8 +9,8 @@
 #define _NGINX_H_INCLUDED_
 
 
-#define nginx_version  1017003
-#define NGINX_VERSION  "1.17.3"
+#define nginx_version  1017004
+#define NGINX_VERSION  "1.17.4"
 #define NGINX_VER  "nginx/" NGINX_VERSION
 
 #ifdef NGX_BUILD
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel


Re: cache: move open to thread pool

2019-08-16 Thread 洪志道
Great job, the patch works well.

On Fri, Aug 16, 2019 at 10:24 PM Roman Arutyunyan  wrote:

> Hi,
>
> On Wed, Jul 24, 2019 at 11:47:19AM +0800, 洪志道 wrote:
> > Hi,  I found an issue with the patch.
> >
> > 1. Reproduce
> > (/usr/local/nginx/html/5M.txt, Just making `ngx_http_set_write_handler`
> be
> > called.)
> >
> > telnet localhost 80
> > GET /5M.txt HTTP/1.1
> > HOST: 1.1.1.1
> >
> > -- response--
> >
> > GET /5M.txt HTTP/1.1
> > HOST: 1.1.1.1
> >
> > --50 error-- /* happened in keepalive */
>
> Thanks for reporting this.
>
> > 2. Reason
> > It happens in the case of keepalive.
> > Since `ngx_http_set_write_handler` is called, c->write is active and
> ready,
> > after receiving http request:
> > ngx_http_static_handler is called
> > {
> > ngx_http_static_send();  // task has been added thread pool, but its
> > event is not complete.
> > r->write_event_handler = ngx_http_static_write_event_handler;
> > }
> > And the epoll write event is triggered. So
> > ngx_http_static_write_event_handler is called again.
> > But task->event.active is true. ngx_thread_task_post() will fail.
> > Throws "task #%ui already active".
>
> The problem only manifests itself with epoll.  After both EPOLLIN and
> EPOLLOUT
> for the same socket are registered in epoll, EPOLLOUT is reported along
> with
> EPOLLIN despite the fact that only the read state of the socket changed
> but the
> write state didn't (socket was and is writable).  This is not a problem by
> itself, but it helped to find the problem with this patch.
>
> While thread task is in progress, calling
> ngx_http_static_write_event_handler()
> resulted in posting the task again, which produced the error.
>
> > 3. Early patch.
> > ```
> > diff -r bd11e3399021 src/http/modules/ngx_http_static_module.c
> > --- a/src/http/modules/ngx_http_static_module.c Tue Oct 30 15:00:49 2018
> > +0300
> > +++ b/src/http/modules/ngx_http_static_module.c Tue Jul 23 23:46:22 2019
> > -0400
> > @@ -318,6 +318,12 @@ ngx_http_static_write_event_handler(ngx_
> >  {
> >  ngx_int_t  rc;
> >
> > +if (r->open_file_info.thread_task != NULL
> > +&& r->open_file_info.thread_task->event.complete == 0)
> > +{
> > +return;
> > +}
> > +
> >  rc = ngx_http_static_send(r);
> >
> >  if (rc != NGX_DONE) {
>
> Similar problem in thread read is prevented by checking the flag r->aio.
> I did the same for thread open (see attachment).
>
> > ```
> >
> > Thanks.
> >
> >
> > On Mon, Jul 22, 2019 at 10:05 PM Roman Arutyunyan 
> wrote:
> >
> > > Hi,
> > >
> > > On Sat, Jul 20, 2019 at 09:44:25AM +0800, 洪志道 wrote:
> > > > > The patch wasn't updated since that but I suspect it could be still
> > > > applied to nginx, maybe with some minor tweaks.
> > > >
> > > > We still appreciate your work.
> > > >
> > > > BTW, are the patches in the attachment up to date?
> > > > If not, can you share the latest patch?
> > >
> > > We had no patches since november 1, 2018.  The last patch is in this
> > > message:
> > >
> > >
> http://mailman.nginx.org/pipermail/nginx-devel/2018-November/011538.html
> > >
> > > > We are happy to apply it and feedback if possible.
> > > > When there are many static files and accesses, do you think it will
> works
> > > > very well?
> > >
> > > We didn't have much feedback so far.  So we would definitely appreciate
> > > your
> > > contribution to testing it.
> > >
> > > > On Sat, Jul 20, 2019 at 2:42 AM Maxim Konovalov 
> wrote:
> > > >
> > > > > Hi hongzhidao,
> > > > >
> > > > > The patch wasn't merged as we didn't see much interest and real
> > > > > technical feedback from potential testers.
> > > > >
> > > > > The code adds additional complexity to the nginx core with all
> > > > > associated costs of maintaining the code virtually forever.  In the
> > > > > same time at this point it brings no measurable value to the
> > > > > community.  At least, we haven't seen any proofs that it does.
> > > > >
> > > > > The patch wasn't updated since that but I suspect it could be still
> > > > > applied to nginx, maybe with some minor tweaks.
> > > > >
> > > > > Maxim
> > > > >
> > > > > On 19/07/2019 18:26, 洪志道 wrote:
> > > > > > Hi.
> > > > > > Will this patch be merged into the main branch?
> > > > > > What is the latest patch? We can help with the test.
> > > > > > Thanks.
> > > > > >
> > > > > > On Sat, Feb 9, 2019 at 6:40 AM Ka-Hing Cheung via nginx-devel
> > > > > > mailto:nginx-devel@nginx.org>> wrote:
> > > > > >
> > > > > > Unfortunately our test colo is not setup to do performance
> > > testing
> > > > > > (the traffic it receives varies too much). We do intend to
> merge
> > > > > > this
> > > > > > to our production colos but there's no timeline yet.
> > > > > >
> > > > > > Yuchen (CC'ed) will be the main contact from now on as today
> is
> > > my
> > > > > > last day at Cloudflare.
> > > > > >
> > > > > > - Ka-Hing
> > > > > >
> > > > > > On Thu, Feb 7, 2019 at 5:39 AM Maxim Konovalov <
> ma...@nginx.com
> > >

Re: cache: move open to thread pool

2019-08-16 Thread Roman Arutyunyan
Hi,

On Wed, Jul 24, 2019 at 11:47:19AM +0800, 洪志道 wrote:
> Hi,  I found an issue with the patch.
> 
> 1. Reproduce
> (/usr/local/nginx/html/5M.txt, Just making `ngx_http_set_write_handler` be
> called.)
> 
> telnet localhost 80
> GET /5M.txt HTTP/1.1
> HOST: 1.1.1.1
> 
> -- response--
> 
> GET /5M.txt HTTP/1.1
> HOST: 1.1.1.1
> 
> --50 error-- /* happened in keepalive */

Thanks for reporting this.

> 2. Reason
> It happens in the case of keepalive.
> Since `ngx_http_set_write_handler` is called, c->write is active and ready,
> after receiving http request:
> ngx_http_static_handler is called
> {
> ngx_http_static_send();  // task has been added thread pool, but its
> event is not complete.
> r->write_event_handler = ngx_http_static_write_event_handler;
> }
> And the epoll write event is triggered. So
> ngx_http_static_write_event_handler is called again.
> But task->event.active is true. ngx_thread_task_post() will fail.
> Throws "task #%ui already active".

The problem only manifests itself with epoll.  After both EPOLLIN and EPOLLOUT
for the same socket are registered in epoll, EPOLLOUT is reported along with
EPOLLIN despite the fact that only the read state of the socket changed but the
write state didn't (socket was and is writable).  This is not a problem by
itself, but it helped to find the problem with this patch.

While thread task is in progress, calling ngx_http_static_write_event_handler()
resulted in posting the task again, which produced the error.

> 3. Early patch.
> ```
> diff -r bd11e3399021 src/http/modules/ngx_http_static_module.c
> --- a/src/http/modules/ngx_http_static_module.c Tue Oct 30 15:00:49 2018
> +0300
> +++ b/src/http/modules/ngx_http_static_module.c Tue Jul 23 23:46:22 2019
> -0400
> @@ -318,6 +318,12 @@ ngx_http_static_write_event_handler(ngx_
>  {
>  ngx_int_t  rc;
> 
> +if (r->open_file_info.thread_task != NULL
> +&& r->open_file_info.thread_task->event.complete == 0)
> +{
> +return;
> +}
> +
>  rc = ngx_http_static_send(r);
> 
>  if (rc != NGX_DONE) {

Similar problem in thread read is prevented by checking the flag r->aio.
I did the same for thread open (see attachment).

> ```
> 
> Thanks.
> 
> 
> On Mon, Jul 22, 2019 at 10:05 PM Roman Arutyunyan  wrote:
> 
> > Hi,
> >
> > On Sat, Jul 20, 2019 at 09:44:25AM +0800, 洪志道 wrote:
> > > > The patch wasn't updated since that but I suspect it could be still
> > > applied to nginx, maybe with some minor tweaks.
> > >
> > > We still appreciate your work.
> > >
> > > BTW, are the patches in the attachment up to date?
> > > If not, can you share the latest patch?
> >
> > We had no patches since november 1, 2018.  The last patch is in this
> > message:
> >
> > http://mailman.nginx.org/pipermail/nginx-devel/2018-November/011538.html
> >
> > > We are happy to apply it and feedback if possible.
> > > When there are many static files and accesses, do you think it will works
> > > very well?
> >
> > We didn't have much feedback so far.  So we would definitely appreciate
> > your
> > contribution to testing it.
> >
> > > On Sat, Jul 20, 2019 at 2:42 AM Maxim Konovalov  wrote:
> > >
> > > > Hi hongzhidao,
> > > >
> > > > The patch wasn't merged as we didn't see much interest and real
> > > > technical feedback from potential testers.
> > > >
> > > > The code adds additional complexity to the nginx core with all
> > > > associated costs of maintaining the code virtually forever.  In the
> > > > same time at this point it brings no measurable value to the
> > > > community.  At least, we haven't seen any proofs that it does.
> > > >
> > > > The patch wasn't updated since that but I suspect it could be still
> > > > applied to nginx, maybe with some minor tweaks.
> > > >
> > > > Maxim
> > > >
> > > > On 19/07/2019 18:26, 洪志道 wrote:
> > > > > Hi.
> > > > > Will this patch be merged into the main branch?
> > > > > What is the latest patch? We can help with the test.
> > > > > Thanks.
> > > > >
> > > > > On Sat, Feb 9, 2019 at 6:40 AM Ka-Hing Cheung via nginx-devel
> > > > > mailto:nginx-devel@nginx.org>> wrote:
> > > > >
> > > > > Unfortunately our test colo is not setup to do performance
> > testing
> > > > > (the traffic it receives varies too much). We do intend to merge
> > > > > this
> > > > > to our production colos but there's no timeline yet.
> > > > >
> > > > > Yuchen (CC'ed) will be the main contact from now on as today is
> > my
> > > > > last day at Cloudflare.
> > > > >
> > > > > - Ka-Hing
> > > > >
> > > > > On Thu, Feb 7, 2019 at 5:39 AM Maxim Konovalov  > > > > > wrote:
> > > > > >
> > > > > > Great.  Thanks for the testing!
> > > > > >
> > > > > > Did you see any measurable perf. metrics changes comparing to
> > your
> > > > > > aio open implementation or comparing to nginx without aio open
> > > > > support?
> > > > > >
> > > > > > We are still waiting for additional input from another t