Option to disable buffering in uwsgi module

[Maxim Dounin]

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