Using rate limitation for files smaller than the defined limit

aris · July 30, 2012, 2:23pm

Hi!

We use nginx rate limiting function but realized that it does not work
for files smaller than the limit specified in limit_rate directive.
Analyzing the source code it turned out that forcing the limit is second
based. We modified the source to be able to rate limit more precisely
and it also works for smaller files.

Here is the patch:
leki@szunyog:~/src/agwlimit/nginx-1.2.2$ diff -u
src/http/ngx_http_write_filter_module.c{.old,}
— src/http/ngx_http_write_filter_module.c.old 2012-07-30
08:32:13.155354399 +0200
+++ src/http/ngx_http_write_filter_module.c 2012-07-30
13:55:01.074823215 +0200
@@ -211,8 +211,15 @@
}

 if (r->limit_rate) {

   limit = r->limit_rate * (ngx_time() - r->start_sec + 1)

           - (c->sent - clcf->limit_rate_after);

   if (ngx_cached_time->msec < r->start_msec) {

       limit = r->limit_rate * (ngx_time() - r->start_sec - 1)

               + r->limit_rate * (ngx_cached_time->msec + 1000 -

r->start_msec) / 1000

               - (c->sent - clcf->limit_rate_after);

```
   } else {
```

       limit = r->limit_rate * (ngx_time() - r->start_sec)

               + r->limit_rate * (ngx_cached_time->msec -

r->start_msec) / 1000

               - (c->sent - clcf->limit_rate_after);

   }

   if (limit <= 0) {
       c->write->delayed = 1;

As we tested the pathced nginx it worked fine. Could we use it in
production or do you see any mistakes we made? Is there any reason you
don’t count on milliseconds?

Regards,
Gabor

Posted at Nginx Forum:

leki75 · July 30, 2012, 7:26pm

Hello!

On Mon, Jul 30, 2012 at 08:22:42AM -0400, leki75 wrote:

src/http/ngx_http_write_filter_module.c{.old,}
   if (ngx_cached_time->msec < r->start_msec) {

Using ngx_cached_time directly is a bad idea. While it likely
will work correctly right now (unless you are really happy and the
expression will be broken in the middle by a timer signal,
assuming timer_resolution used on linux), it’s quite likely to
break in the future.

Using ngx_timeofday() instead should be better.

       limit = r->limit_rate * (ngx_time() - r->start_sec - 1)

               + r->limit_rate * (ngx_cached_time->msec + 1000 -

r->start_msec) / 1000

               - (c->sent - clcf->limit_rate_after);

```
   } else {
```

       limit = r->limit_rate * (ngx_time() - r->start_sec)

               + r->limit_rate * (ngx_cached_time->msec -

r->start_msec) / 1000

               - (c->sent - clcf->limit_rate_after);

This basically limits to 0 at request start unless
limit_rate_after is set to something non-zero, resulting in a
first byte delay for all requests.

   }

   if (limit <= 0) {
       c->write->delayed = 1;
As we tested the pathced nginx it worked fine. Could we use it in
production or do you see any mistakes we made?

The above might be ok for your particular use case, though
problems outlined above (and below) might cause suboptimal
performance.

Is there any reason you don’t count on milliseconds?

Current code uses seconds as it’s easier to handle, faster
(implies less syscalls and less packets over network) and provides
some inherent size for a first write.

With milliseconds and a 4k/s limit (as used in example in docs,
see Module ngx_http_core_module) there will be 4 byte write each
millisecond, which is quite inefficient.

Maxim D.

leki75 · July 31, 2012, 6:21pm

Hello!

On Tue, Jul 31, 2012 at 11:18:18AM -0400, leki75 wrote:

[…]

Can we avoid network segmentation by setting the following configuration
directives?
tcp_nodelay off;
tcp_nopush on;

No.

Do you have any other suggestions how to rate limit objects, which size
is smaller than specified in limit_rate, other way?

You may try to delay last byte of a response (or something like
this, ideally - just a partially filled last packet) instead.

Alternatively, you may introduce some granularity for limits and
always allow a chunk of data (something like one packet at least)
if at least one byte is allowed. This should mitigate problems
introduced by millisecond resolution.

Maxim D.

leki75 · July 31, 2012, 5:19pm

Dear Maxim,

thank you for your reply. I changed the code you suggested:

changed ngx_cached_time to ngx_timeofday()
added 1ms to send some bytes at request start

— src/http/ngx_http_write_filter_module.c 2012-01-18
16:07:43.000000000 +0100
+++ src/http/ngx_http_write_filter_module.c.new 2012-07-31
16:38:11.074836346 +0200
@@ -211,8 +211,17 @@
}

 if (r->limit_rate) {

   limit = r->limit_rate * (ngx_time() - r->start_sec + 1)

           - (c->sent - clcf->limit_rate_after);

```
   ngx_time_t  *t = ngx_timeofday();
```
```
   if (t->msec >= r->start_msec) {
```

       limit = r->limit_rate * (t->sec - r->start_sec)

               + r->limit_rate * (t->msec - r->start_msec + 1) /

1000

               - (c->sent - clcf->limit_rate_after);

```
   } else {
```

       limit = r->limit_rate * (t->sec - r->start_sec - 1)

               + r->limit_rate * (t->msec - r->start_msec + 1001)

/ 1000

               - (c->sent - clcf->limit_rate_after);

   }

   if (limit <= 0) {
       c->write->delayed = 1;

Can we avoid network segmentation by setting the following configuration
directives?
tcp_nodelay off;
tcp_nopush on;

Do you have any other suggestions how to rate limit objects, which size
is smaller than specified in limit_rate, other way?

Regards,
Gabor

Posted at Nginx Forum:

leki75 · September 4, 2012, 12:55pm

Hello!

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

off_t size, sent, nsent, limit, nlimit;
```
   if (last && size == 1) {
```
```
   }
```
```
   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.

        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 D. [email protected]

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 D.

leki75 · August 27, 2012, 2:00pm

Dear Maxim,

thank you for your suggestions. Analyzing the code the following turned
out
about http_write_filter:

it is able to buffer output (eg. postpone_output)
can delay response before sending bytes (limit <= 0)
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)
   {

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;
    }

Thanks for your help.

Regards,
Gabor

Posted at Nginx Forum: