cache: move open to thread pool
洪志道
hongzhidao at gmail.com
Wed Jul 24 03:47:19 UTC 2019
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 */
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".
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) {
```
Thanks.
On Mon, Jul 22, 2019 at 10:05 PM Roman Arutyunyan <arut at nginx.com> 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 <maxim at nginx.com> 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
> > > > <nginx-devel at nginx.org <mailto:nginx-devel at 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 <maxim at nginx.com
> > > > <mailto:maxim at nginx.com>> 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 tester,
> who
> > > > > expressed interest before.
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Maxim
> > > > >
> > > > > On 07/02/2019 00:19, Ka-Hing Cheung wrote:
> > > > > > This has been running in our test colo for the past week
> > > > with no ill effects.
> > > > > >
> > > > > > On Wed, Jan 23, 2019 at 4:39 AM Maxim Konovalov
> > > > <maxim at nginx.com <mailto:maxim at nginx.com>> wrote:
> > > > > >>
> > > > > >> Hi Ka-Hing,
> > > > > >>
> > > > > >> Roman told me that the delta is because of your changes.
> > > > > >>
> > > > > >> Thanks for your time on that. Waiting for your testing
> > > > results.
> > > > > >>
> > > > > >> Maxim
> > > > > >>
> > > > > >> On 22/01/2019 22:34, Ka-Hing Cheung via nginx-devel wrote:
> > > > > >>> I spoke too soon, just realized our test colo is running
> > > > the patches
> > > > > >>> without aio_open on. Flipping that switch now.
> > > > > >>>
> > > > > >>> Also, including the patch that we applied on top in
> > > > addition to the
> > > > > >>> massaging we did to resolve conflicts. I haven't dug too
> > > > deep to see
> > > > > >>> if stock nginx also requires similar changes or they are
> only
> > > > > >>> necessary because of our other nginx changes:
> > > > > >>>
> > > > > >> [...]
> > > > > >>
> > > > > >> --
> > > > > >> Maxim Konovalov
> > > > >
> > > > >
> > > > > --
> > > > > Maxim Konovalov
> > > > _______________________________________________
> > > > nginx-devel mailing list
> > > > nginx-devel at nginx.org <mailto:nginx-devel at nginx.org>
> > > > http://mailman.nginx.org/mailman/listinfo/nginx-devel
> > > >
> > > >
> > > > _______________________________________________
> > > > nginx-devel mailing list
> > > > nginx-devel at nginx.org
> > > > http://mailman.nginx.org/mailman/listinfo/nginx-devel
> > > >
> > >
> > >
> > > --
> > > Maxim Konovalov
> > >
>
> > _______________________________________________
> > nginx-devel mailing list
> > nginx-devel at nginx.org
> > http://mailman.nginx.org/mailman/listinfo/nginx-devel
>
>
> --
> Roman Arutyunyan
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20190724/4d1cc9d6/attachment.html>
More information about the nginx-devel
mailing list