[PATCH] Option to disable buffering in uwsgi module

Peter Smit peter at smitmail.eu
Fri Sep 30 10:00:12 UTC 2011


Attached and inlined is a patch which makes it possible to disable
buffering for the fastcgi,scgi and uwsgi modules in the following
ways:

- By using a {module}_buffering on|off configuration value
- By using the X-Accel-Buffering header

This is essential for any apps that want to do comet / websocket style
communications.



Index: ngx_http_fastcgi_module.c
===================================================================
--- ngx_http_fastcgi_module.c	(revision 4157)
+++ ngx_http_fastcgi_module.c	(working copy)
@@ -228,6 +228,13 @@
       offsetof(ngx_http_fastcgi_loc_conf_t, upstream.store_access),
       NULL },

+    { ngx_string("fastcgi_buffering"),
+      NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_FLAG,
+      ngx_conf_set_flag_slot,
+      NGX_HTTP_LOC_CONF_OFFSET,
+      offsetof(ngx_http_fastcgi_loc_conf_t, upstream.buffering),
+      NULL },
+
     { ngx_string("fastcgi_ignore_client_abort"),
       NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_FLAG,
       ngx_conf_set_flag_slot,
@@ -602,7 +609,7 @@
     u->abort_request = ngx_http_fastcgi_abort_request;
     u->finalize_request = ngx_http_fastcgi_finalize_request;

-    u->buffering = 1;
+    u->buffering = flcf->upstream.buffering;

     u->pipe = ngx_pcalloc(r->pool, sizeof(ngx_event_pipe_t));
     if (u->pipe == NULL) {
@@ -2072,6 +2079,8 @@
     conf->catch_stderr = NGX_CONF_UNSET_PTR;

     conf->keep_conn = NGX_CONF_UNSET;
+
+    conf->upstream.change_buffering = 1;

     ngx_str_set(&conf->upstream.module, "fastcgi");

Index: ngx_http_scgi_module.c
===================================================================
--- ngx_http_scgi_module.c	(revision 4157)
+++ ngx_http_scgi_module.c	(working copy)
@@ -96,6 +96,13 @@
       offsetof(ngx_http_scgi_loc_conf_t, upstream.store_access),
       NULL },

+    { ngx_string("scgi_buffering"),
+      NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_FLAG,
+      ngx_conf_set_flag_slot,
+      NGX_HTTP_LOC_CONF_OFFSET,
+      offsetof(ngx_http_scgi_loc_conf_t, upstream.buffering),
+      NULL },
+
     { ngx_string("scgi_ignore_client_abort"),
       NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_FLAG,
       ngx_conf_set_flag_slot,
@@ -412,7 +419,7 @@
     u->abort_request = ngx_http_scgi_abort_request;
     u->finalize_request = ngx_http_scgi_finalize_request;

-    u->buffering = 1;
+    u->buffering = scf->upstream.buffering;

     u->pipe = ngx_pcalloc(r->pool, sizeof(ngx_event_pipe_t));
     if (u->pipe == NULL) {
@@ -1038,6 +1045,8 @@
     /* "scgi_cyclic_temp_file" is disabled */
     conf->upstream.cyclic_temp_file = 0;

+    conf->upstream.change_buffering = 1;
+
     ngx_str_set(&conf->upstream.module, "scgi");

     return conf;
Index: ngx_http_uwsgi_module.c
===================================================================
--- ngx_http_uwsgi_module.c	(revision 4157)
+++ ngx_http_uwsgi_module.c	(working copy)
@@ -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_FLAG,
+      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;

     u->pipe = ngx_pcalloc(r->pool, sizeof(ngx_event_pipe_t));
     if (u->pipe == NULL) {
@@ -1091,6 +1098,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;



On Fri, Sep 30, 2011 at 11:05 AM, Peter Smit <peter at smitmail.eu> 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?
>
>
>
>
> 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
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: buffering_uwsgi_scgi_fastcgi.patch
Type: text/x-patch
Size: 3698 bytes
Desc: not available
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20110930/25bd6137/attachment.bin>


More information about the nginx-devel mailing list