Re: cache: move open to thread pool

2021-09-09 Thread Roman Arutyunyan
Hi Noam, On Thu, Sep 02, 2021 at 07:16:55PM +0300, Noam Cvikel wrote: > Hello, > > Didn't see a way to get into the thread after joining the list, but > hopefully this still replies correctly. > > First of all thank you for the prompt response and updated patch. > We implemented the updated

Re: cache: move open to thread pool

2021-09-02 Thread Noam Cvikel
Hello, Didn't see a way to get into the thread after joining the list, but hopefully this still replies correctly. First of all thank you for the prompt response and updated patch. We implemented the updated async open patch, and started testing it in our lab. The static module part of the patch

Re: cache: move open to thread pool

2021-08-03 Thread Roman Arutyunyan
Hi, On Thu, Nov 01, 2018 at 07:08:10PM +0300, Maxim Konovalov wrote: > On 01/11/2018 15:30, Roman Arutyunyan wrote: > > Hi, > > > > On Thu, Oct 04, 2018 at 12:32:26PM +0300, Roman Arutyunyan wrote: > >> Hi, > >> > >> On Thu, Sep 13, 2018 at 09:10:23PM +0300, Maxim Dounin wrote: > >>> Hello! >

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` >

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

Re: cache: move open to thread pool

2019-07-23 Thread 洪志道
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

Re: cache: move open to thread pool

2019-07-22 Thread 洪志道
Thanks again. 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

Re: cache: move open to thread pool

2019-07-19 Thread 洪志道
> 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 are happy to apply it and feedback if possible. When

Re: cache: move open to thread pool

2019-07-19 Thread Maxim Konovalov
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

Re: cache: move open to thread pool

2019-02-08 Thread Ka-Hing Cheung via nginx-devel
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

Re: cache: move open to thread pool

2019-02-07 Thread Maxim Konovalov
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

Re: cache: move open to thread pool

2019-02-06 Thread Ka-Hing Cheung via nginx-devel
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 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

Re: cache: move open to thread pool

2019-01-23 Thread Maxim Konovalov
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.

Re: cache: move open to thread pool

2019-01-22 Thread Ka-Hing Cheung via nginx-devel
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

Re: cache: move open to thread pool

2019-01-22 Thread Ka-Hing Cheung via nginx-devel
Hi Maxim and Roman, We've been running a version of your patches in our test colo for the past week with no ill effects, with the caveat that it's difficult to measure performance impacts in our test colo because of varying traffic. We had to massage the patches a bit because we don't run vanilla

Re: cache: move open to thread pool

2019-01-14 Thread Ka-Hing Cheung via nginx-devel
We didn't get to it last year because of other priorities but this is being worked on. We don't use a vanilla nginx so it takes effort to integrate your patches. Having a version of this in upstream nginx is important to us and to me personally. On Thu, Jan 10, 2019 at 12:30 AM Maxim Konovalov

Re: cache: move open to thread pool

2019-01-10 Thread Maxim Konovalov
So guys, are you really interested in this work beyond shiny marketing blog posts? On 05/12/2018 12:43, Maxim Konovalov wrote: > Hello, > > just a reminder that we are looking for a tester for these patches. > > Thanks, > > Maxim > > On 16/11/2018 12:10, Maxim Konovalov wrote: >> Thanks,

Re: cache: move open to thread pool

2018-11-16 Thread Maxim Konovalov
Thanks, Ka-Hing, we'll wait for your feedback. On 16/11/2018 01:53, Ka-Hing Cheung wrote: > Hi, > > I didn't forget about this. I am pretty swamped at the moment and > there's a holiday freeze coming up. Will get to his in December. > > - Ka-Hing > > On Thu, Nov 8, 2018 at 6:19 AM Maxim

Re: cache: move open to thread pool

2018-11-15 Thread Ka-Hing Cheung via nginx-devel
Hi, I didn't forget about this. I am pretty swamped at the moment and there's a holiday freeze coming up. Will get to his in December. - Ka-Hing On Thu, Nov 8, 2018 at 6:19 AM Maxim Konovalov wrote: > > Hi Ka-Hing, > > would you mind to test Roman's most recent patches that add > "aio_open"

Re: cache: move open to thread pool

2018-11-08 Thread Maxim Konovalov
Hi Ka-Hing, would you mind to test Roman's most recent patches that add "aio_open" directive? http://mailman.nginx.org/pipermail/nginx-devel/2018-November/011538.html We are looking for overall performance and stability metrics and feedback. Much appreciated, Maxim -- Maxim Konovalov

Re: cache: move open to thread pool

2018-11-01 Thread Maxim Konovalov
On 01/11/2018 15:30, Roman Arutyunyan wrote: > Hi, > > On Thu, Oct 04, 2018 at 12:32:26PM +0300, Roman Arutyunyan wrote: >> Hi, >> >> On Thu, Sep 13, 2018 at 09:10:23PM +0300, Maxim Dounin wrote: >>> Hello! >>> >>> On Tue, Sep 04, 2018 at 04:58:05PM -0700, Ka-Hing Cheung via nginx-devel >>>

Re: cache: move open to thread pool

2018-11-01 Thread Roman Arutyunyan
Hi, On Thu, Oct 04, 2018 at 12:32:26PM +0300, Roman Arutyunyan wrote: > Hi, > > On Thu, Sep 13, 2018 at 09:10:23PM +0300, Maxim Dounin wrote: > > Hello! > > > > On Tue, Sep 04, 2018 at 04:58:05PM -0700, Ka-Hing Cheung via nginx-devel > > wrote: > > > > > On Mon, Sep 3, 2018 at 9:09 AM, Maxim

Re: cache: move open to thread pool

2018-10-04 Thread Roman Arutyunyan
Hi, On Thu, Sep 13, 2018 at 09:10:23PM +0300, Maxim Dounin wrote: > Hello! > > On Tue, Sep 04, 2018 at 04:58:05PM -0700, Ka-Hing Cheung via nginx-devel > wrote: > > > On Mon, Sep 3, 2018 at 9:09 AM, Maxim Dounin wrote: [..] Here's another approach to thread open. This time it's 4 patches:

Re: cache: move open to thread pool

2018-09-13 Thread Maxim Dounin
Hello! On Tue, Sep 04, 2018 at 04:58:05PM -0700, Ka-Hing Cheung via nginx-devel wrote: > On Mon, Sep 3, 2018 at 9:09 AM, Maxim Dounin wrote: > > The biggest problem with in-place solution is that it requires > > in-place coding for all the places where we want to use aio_open. > > Currently we

Re: cache: move open to thread pool

2018-09-04 Thread Ka-Hing Cheung via nginx-devel
On Mon, Sep 3, 2018 at 9:09 AM, Maxim Dounin wrote: > The biggest problem with in-place solution is that it requires > in-place coding for all the places where we want to use aio_open. > Currently we use ngx_open_cached_file() to open files everywhere, > and it would be a good idea to preserve it

Re: cache: move open to thread pool

2018-09-03 Thread Maxim Dounin
Hello! On Mon, Aug 27, 2018 at 02:38:36PM -0700, Ka-Hing Cheung via nginx-devel wrote: > Thanks for all the feedback! This is an updated version of the patch. > There may still be some coding style issues (which will be addressed), > but should address most of the comments other than these two:

RE: cache: move open to thread pool

2018-08-28 Thread Eran Kornblau
Hi, > Hi Eran, > > Happy to see that we are not the only place where we find open to be a > problem. I took a look at your module and while the approach is sound, I > decided the complexity doesn't seem to be worth it. Hope to see some numbers > of how your module works at scale. > > -

Re: cache: move open to thread pool

2018-08-27 Thread Ka-Hing Cheung via nginx-devel
Hi Eran, Happy to see that we are not the only place where we find open to be a problem. I took a look at your module and while the approach is sound, I decided the complexity doesn't seem to be worth it. Hope to see some numbers of how your module works at scale. - Ka-Hing On Wed, Aug 8, 2018

Re: cache: move open to thread pool

2018-08-27 Thread Ka-Hing Cheung via nginx-devel
Thanks for all the feedback! This is an updated version of the patch. There may still be some coding style issues (which will be addressed), but should address most of the comments other than these two: > - The code bypasses open file cache, and uses a direct call > in the http cache code

Re: cache: move open to thread pool

2018-08-10 Thread Maxim Dounin
Hello! On Thu, Aug 09, 2018 at 02:43:19PM -0700, Ka-Hing Cheung via nginx-devel wrote: > On Wed, Aug 8, 2018 at 11:16 AM, Maxim Dounin wrote: > > > > We've thought about offloading more operations to thread pools, > > and that's basically why "aio_write" is a separate directive, so > > more

Re: cache: move open to thread pool

2018-08-09 Thread Ka-Hing Cheung via nginx-devel
Very good feedback, we will look into implementing some of them at least. On Wed, Aug 8, 2018 at 11:16 AM, Maxim Dounin wrote: > > We've thought about offloading more operations to thread pools, > and that's basically why "aio_write" is a separate directive, so > more directives like "aio_" can

RE: cache: move open to thread pool

2018-08-08 Thread Eran Kornblau
Hi, > > - The code bypasses open file cache, and uses a direct call > in the http cache code instead. While it might be ok in your > setup, it looks like an incomplete solution from the generic point > of view. A better solution would be to introduce a generic > interface in

Re: cache: move open to thread pool

2018-08-08 Thread Maxim Dounin
Hello! On Tue, Aug 07, 2018 at 02:26:01PM -0700, Ka-Hing Cheung via nginx-devel wrote: > commit 0bcfefa040abdff674047c15701d237ff16a5f2b > Author: Ka-Hing Cheung > Date: Fri Aug 3 13:37:58 2018 -0700 > > move open to thread pool > > At cloudflare we found that open() can block a