Option to disable buffering in uwsgi module

Fri Sep 30 10:04:52 UTC 2011

Hello!

On Fri, Sep 30, 2011 at 11:05:54AM +0300, Peter Smit wrote:

> I am now getting around to create this patch. Would it be a good idea
> to have the buffering option for uwsgi, scgi and fastcgi? Would you
> like all three in different patches or in one?

It's ok to have uwsgi and scgi buffering in a single patch.

If you are going to work on fastcgi, please use separate patch.  
It will be much more complex one though.

Maxim Dounin

> 
> 
> 
> 
> 2011/9/23 Maxim Dounin <mdounin at mdounin.ru>:
> > Hello!
> >
> > On Fri, Sep 23, 2011 at 01:51:20PM +0300, Peter Smit wrote:
> >
> >> On Fri, Sep 23, 2011 at 1:31 PM, Maxim Dounin <mdounin at mdounin.ru> wrote:
> >> >
> >> > Hello!
> >> >
> >> > On Fri, Sep 23, 2011 at 09:24:29AM +0300, Peter Smit wrote:
> >> >
> >> > > >
> >> > > > On Thu, Aug 04, 2011 at 01:32:14PM +0300, Peter Smit wrote:
> >> > > > > (This is my first message to this list, please let me know if I'm
> >> > > > > doing something wrong!)
> >> > > > >
> >> > > > > In developing a comet style application in a uwsgi/nginx setup I
> >> > > > > noticed that nginx always buffers the response when uwsgi_pass is
> >> > > > > used.
> >> > > > >
> >> > > > > I'm not sure or there is any particular reason why this is done,
> >> > > > > except for the fact that the uwsgi code is originally based on the
> >> > > > > fastcgi module where indeed buffering is unavoidable. I think however
> >> > > > > that it makes sense to give the option of disabling buffering with
> >> > > > > uwsgi.
> >> > > > >
> >> > > > > I actually already went ahead and wrote a patch that does exactly
> >> > > > > this. It introduces a uwsgi_buffering flag and now adheres to the
> >> > > > > "X-Accel-Buffering" header. I have only limited capabilities to test
> >> > > > > this patch, but for me it does exactly that, disabling the buffer.
> >> > > > >
> >> > > > > Could some of you review this patch and if it is ok, could it be
> >> > > > > introduced in nginx?
> >> > > > >
> >> > > > > I made the patch on the 1.1.0 source. I attached it and included it
> >> > > > > inline below this message. Let me know if I should give it in a
> >> > > > > different format.
> >> > > >
> >> > > > Thank you for idea, it seems unbuffered proxying should work for uwsgi
> >> > > > as well as for scgi, since they both use simple protocol, that is, no
> >> > > > protocol at all :) for body.
> >> > > >
> >> > > >
> >> > > >
> >> > > How does the process go further? I noticed that the patch is not included
> >> > > yet. Are you waiting for someone to step up to make a patch also for scgi,
> >> > > or are there other reasons for not applying the patch yet?
> >> >
> >> > The patch wasn't yet considered due to a number of upstream
> >> > infrasture changes related to upstream keepalive support.  Now
> >> > with upstream keepalive committed we may proceed with the patch.
> >> >
> >> > Could you please provide patch for scgi as well?
> >> >
> >> > And please see review below, there are some minor nits which
> >> > should be addressed.
> >> >
> >>
> >> End of next week I will look to this and maybe send in a new patch for
> >> both uwsgi and scgi. Can I checkout somewhere the svn tip to make my
> >> patch against? Or should I take the latest release code?
> >
> > You can checkout at svn://svn.nginx.org/nginx/trunk.  Though it's
> > ok to use latest development release as well.
> >
> > Maxim Dounin
> >
> >
> >>
> >> > [...]
> >> >
> >> > > --- nginx-1.1.0/src/http/modules/ngx_http_uwsgi_module.c        2011-07-29
> >> > > 18:33:03.000000000 +0300
> >> > > +++ nginx-1.1.0-uwsgi_buffering/src/http/modules/ngx_http_uwsgi_module.c
> >> > >    2011-08-04
> >> > > 13:16:54.381528459 +0300
> >> > > @@ -123,6 +123,13 @@
> >> > >       offsetof(ngx_http_uwsgi_loc_conf_t, upstream.store_access),
> >> > >       NULL },
> >> > >
> >> > > +    { ngx_string("uwsgi_buffering"),
> >> > > +
> >> > >  NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE123,
> >> >
> >> > This shouldn't be TAKE123, but NGX_CONF_FLAG instead.
> >> >
> >> > > +      ngx_conf_set_flag_slot,
> >> > > +      NGX_HTTP_LOC_CONF_OFFSET,
> >> > > +      offsetof(ngx_http_uwsgi_loc_conf_t, upstream.buffering),
> >> > > +      NULL },
> >> > > +
> >> > >     { ngx_string("uwsgi_ignore_client_abort"),
> >> > >       NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_FLAG,
> >> > >       ngx_conf_set_flag_slot,
> >> > > @@ -445,7 +452,7 @@
> >> > >     u->abort_request = ngx_http_uwsgi_abort_request;
> >> > >     u->finalize_request = ngx_http_uwsgi_finalize_request;
> >> > >
> >> > > -    u->buffering = 1;
> >> > > +    u->buffering = uwcf->upstream.buffering;
> >> >
> >> > It looks like posted patch suffers from whitespace corruption,
> >> > likely by mail client.  You may want to attach it instead.
> >> >
> >> > >
> >> > >     u->pipe = ngx_pcalloc(r->pool, sizeof(ngx_event_pipe_t));
> >> > >     if (u->pipe == NULL) {
> >> > > @@ -1083,6 +1090,8 @@
> >> > >     /* "uwsgi_cyclic_temp_file" is disabled */
> >> > >     conf->upstream.cyclic_temp_file = 0;
> >> > >
> >> > > +    conf->upstream.change_buffering = 1;
> >> > > +
> >> > >     ngx_str_set(&conf->upstream.module, "uwsgi");
> >> > >
> >> > >     return conf;
> >> >
> >> > (just for record to myself: configuration merge for
> >> > upstream.buffering is already present in both uwsgi and scgi
> >> > modules, not needed in the patch)
> >> >
> >> > Maxim Dounin
> >> >
> >> > _______________________________________________
> >> > nginx-devel mailing list
> >> > 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
> >
> > _______________________________________________
> > nginx-devel mailing list
> > 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