Using rate limitation for files smaller than the defined limit.

[Maxim Dounin]

Hello!

On Wed, Aug 22, 2012 at 09:24:03AM -0400, leki75 wrote:

> Dear Maxim,
> 
> thank you for your suggestions. Analyzing the code the following turned out
> about http_write_filter:
> 1. it is able to buffer output (eg. postpone_output)
> 2. can delay response before sending bytes (limit <= 0)
> 3. delays response after sending bytes (nsent - sent)
> 
> As you mentioned we delay sending the last byte of the response and only do
> millisecond calculation when sending it.
> 
> $ cat limit_rate.patch
> --- src/http/ngx_http_write_filter_module.c     2012-01-18
> 16:07:43.000000000 +0100
> +++ src/http/ngx_http_write_filter_module.c     2012-08-22
> 12:44:03.862873715 +0200
> @@ -47,9 +47,10 @@
>  ngx_int_t
>  ngx_http_write_filter(ngx_http_request_t *r, ngx_chain_t *in)
>  {
> -    off_t                      size, sent, nsent, limit;
> +    off_t                      size, sent, nsent, limit, nlimit;
>      ngx_uint_t                 last, flush;
>      ngx_msec_t                 delay;
> +    ngx_time_t                *t;
>      ngx_chain_t               *cl, *ln, **ll, *chain;
>      ngx_connection_t          *c;
>      ngx_http_core_loc_conf_t  *clcf;
> @@ -214,6 +215,23 @@
>          limit = r->limit_rate * (ngx_time() - r->start_sec + 1)
>                  - (c->sent - clcf->limit_rate_after);
>  
> +        if (last && size == 1) {
> +            t = ngx_timeofday();
> +
> +            if (t->msec < r->start_msec) {
> +                t->sec--;
> +                t->msec += 1000;
> +            }
> +
> +            nlimit = r->limit_rate * (t->sec - r->start_sec)
> +                     + r->limit_rate * (t->msec - r->start_msec) / 1000
> +                     - (c->sent + size - clcf->limit_rate_after);
> +
> +            if (nlimit <= 0) {
> +                limit = nlimit;
> +            }
> +        }
> +
>          if (limit <= 0) {
>              c->write->delayed = 1;
>              ngx_add_timer(c->write,
> @@ -224,6 +242,12 @@
>              return NGX_AGAIN;
>          }
>  
> +        if (last && limit > size - 1) {
> +            if (size > 1) {
> +                limit = size - 1;
> +            }
> +        }
> +
>          if (clcf->sendfile_max_chunk
>              && (off_t) clcf->sendfile_max_chunk < limit)
>          {

One obvious problem I see that the patch won't delay last byte if 
the last flag wasn't yet received by the writer filter.  This may 
e.g. happen when returning proxied respone as last buffer is sent 
separately there, from ngx_http_upstream_finalize_request().

You may also want to avoid delay of a last byte (and a separate 
syscall for it) if there is no reason to.

Otherwise it should work and looks much better than previous one.

> It also turned out that setting sendfile_max_chunk to a small enough value
> is also a solution for our problem, but this patch also works with default 
> sendfile_max_chunk = 0 setting.
> 
> Anyway, in nginx 1.2.3 source we found this:
>     if (size == 0 && !(c->buffered & NGX_LOWLEVEL_BUFFERED)) {
>         if (last) {
>             r->out = NULL;
>             c->buffered &= ~NGX_HTTP_WRITE_BUFFERED;
> 
>             return NGX_OK;
>         }
> 
>         if (flush) {
>             do {
>                 r->out = r->out->next;
>             } while (r->out);
> 
>             c->buffered &= ~NGX_HTTP_WRITE_BUFFERED;
> 
>             return NGX_OK;
>         }
> 
> Instead we could use this if I am right:
>     if (size == 0 && !(c->buffered & NGX_LOWLEVEL_BUFFERED)) {
>         if (last || flush) {
>             r->out = NULL;
>             c->buffered &= ~NGX_HTTP_WRITE_BUFFERED;
> 
>             return NGX_OK;
>         }

Yes, thank you, the loop is for sure unneeded as is.  It probably 
was here as an incomple code to free chain links, the following 
patch should be ok (and it saves some memory in some special 
cases):

# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1346755968 -14400
# Node ID ab5db32b035b8cd6babc43553b21e0fd01192e7e
# Parent  190da901d41edc7e07c81b9588bd24254370c591
Http write filter: replaced unneeded loop with one to free chains.

Noted by Gabor Lekeny.

diff --git a/src/http/ngx_http_write_filter_module.c b/src/http/ngx_http_write_filter_module.c
--- a/src/http/ngx_http_write_filter_module.c
+++ b/src/http/ngx_http_write_filter_module.c
@@ -185,23 +185,19 @@ ngx_http_write_filter(ngx_http_request_t
     }
 
     if (size == 0 && !(c->buffered & NGX_LOWLEVEL_BUFFERED)) {
-        if (last) {
+        if (last || flush) {
+            for (cl = r->out; cl; /* void */) {
+                ln = cl;
+                cl = cl->next;
+                ngx_free_chain(r->pool, ln);
+            }
+
             r->out = NULL;
             c->buffered &= ~NGX_HTTP_WRITE_BUFFERED;
 
             return NGX_OK;
         }
 
-        if (flush) {
-            do {
-                r->out = r->out->next;
-            } while (r->out);
-
-            c->buffered &= ~NGX_HTTP_WRITE_BUFFERED;
-
-            return NGX_OK;
-        }
-
         ngx_log_error(NGX_LOG_ALERT, c->log, 0,
                       "the http output chain is empty");
 


Maxim Dounin