Is the limit_rate per tcp session or per HTTP request?

[Stefan Hellkvist]

Hi again,

Just to follow up on this one:

I decided to try the route of avoiding to reset "c->sent" for each request
and it does seem to work. To do this I also needed to store the start time
of the ngx_connection to make the rate calculations correct for my use
case. The patch, which is attached, breaks the previous behavior of
limit_rate as it puts limit_rate and limit_rate_after on the tcp session
instead of per request.

As mentioned, the patch seems to solve my particular use case (I can put
limit_rate_after on the tcp session and therefore do rate limitation on
pipelined requests) but I do not claim that it is a general solution that
does not break other things. I include it here anyway in case someone else
is interested. I do not suggest that anyone adds it to nginx for real.

/Stefan



On Tue, Dec 22, 2015 at 9:46 AM, Stefan Hellkvist <hellkvist at gmail.com>
wrote:

>
> Interesting! So perhaps a quick fix for my current use case would be to
> avoid resetting the "sent bytes” on each request? In that case the limit
> will be counted per socket rather than request. Probably not a generic
> solution that everybody would like, as it probably breaks other use cases,
> but perhaps something I can quickly try out on a private branch.
>
>
> That will break limit_rate.
>
> The other peculiarity of the current implementation is that it limits
> the average rate, and the average is calculated by this formula:
>
>    rate = bytes_sent / (current_time - request_start_time)
>
> You may have better luck with the patch below (untested):
>
> diff -r def9c9c9ae05 -r 9e66c0bf7efd
> src/http/ngx_http_write_filter_module.c
> --- a/src/http/ngx_http_write_filter_module.c   Sat Dec 12 10:32:58 2015
> +0300
> +++ b/src/http/ngx_http_write_filter_module.c   Mon Dec 21 16:59:07 2015
> +0300
> @@ -219,7 +219,7 @@ ngx_http_write_filter(ngx_http_request_t
>     }
>
>     if (r->limit_rate) {
> -        if (r->limit_rate_after == 0) {
> +        if (c->requests == 1 && r->limit_rate_after == 0) {
>             r->limit_rate_after = clcf->limit_rate_after;
>         }
>
>
> Thanks for the patch. I tried it however and it does not seem to achieve
> what we want. The patch, as I understand it, only seem to make the
> limit_rate_after config be active on the first request in the pipeline. In
> our case is always a small playlist file (an HLS session starts by loading
> a playlist .m3u8-file) which is always less than the limit_rate_after limit
> that we wanted to act on the whole TCP session, so this has no affect on
> the larger video files that are requested after the first request in the
> pipeline - they will always be rate limited even the first chunks that fit
> under the limit_rate_after border.
>
> What we need is that the sent data counter and the limit_rate_after work
> on the TCP session and not per request and this patch does not seem to
> achieve that unfortunately.
>
> Perhaps another approach, if we do not want to touch the c->sent behaviour
> and keep it per request, would be to decrement the limit_rate_after with
> the total number of bytes sent within this TCP-session (which would mean we
> would have to have a separate counter for that).
>
> Thanks anyway. I’ll see what I can come up with…
>
>
>
> /Stefan
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20151222/d0572313/attachment.html>
-------------- next part --------------
diff -r 78b4e10b4367 src/core/ngx_connection.c
--- a/src/core/ngx_connection.c	Thu Dec 17 16:39:15 2015 +0300
+++ b/src/core/ngx_connection.c	Tue Dec 22 15:11:42 2015 +0100
@@ -946,6 +946,7 @@
 ngx_connection_t *
 ngx_get_connection(ngx_socket_t s, ngx_log_t *log)
 {
+    ngx_time_t         *tp;
     ngx_uint_t         instance;
     ngx_event_t       *rev, *wev;
     ngx_connection_t  *c;
@@ -992,6 +993,9 @@
     c->fd = s;
     c->log = log;
 
+    tp = ngx_timeofday();
+    c->start_sec = tp->sec;
+
     instance = rev->instance;
 
     ngx_memzero(rev, sizeof(ngx_event_t));
diff -r 78b4e10b4367 src/core/ngx_connection.h
--- a/src/core/ngx_connection.h	Thu Dec 17 16:39:15 2015 +0300
+++ b/src/core/ngx_connection.h	Tue Dec 22 15:11:42 2015 +0100
@@ -193,6 +193,8 @@
 #if (NGX_THREADS)
     ngx_thread_task_t  *sendfile_task;
 #endif
+
+    time_t             start_sec;
 };
 
 
diff -r 78b4e10b4367 src/http/ngx_http_request.c
--- a/src/http/ngx_http_request.c	Thu Dec 17 16:39:15 2015 +0300
+++ b/src/http/ngx_http_request.c	Tue Dec 22 15:11:42 2015 +0100
@@ -2934,7 +2934,6 @@
 
         c->data = r;
 
-        c->sent = 0;
         c->destroyed = 0;
 
         if (rev->timer_set) {
@@ -3187,7 +3186,6 @@
         return;
     }
 
-    c->sent = 0;
     c->destroyed = 0;
 
     ngx_del_timer(rev);
diff -r 78b4e10b4367 src/http/ngx_http_write_filter_module.c
--- a/src/http/ngx_http_write_filter_module.c	Thu Dec 17 16:39:15 2015 +0300
+++ b/src/http/ngx_http_write_filter_module.c	Tue Dec 22 15:11:42 2015 +0100
@@ -223,7 +223,7 @@
             r->limit_rate_after = clcf->limit_rate_after;
         }
 
-        limit = (off_t) r->limit_rate * (ngx_time() - r->start_sec + 1)
+        limit = (off_t) r->limit_rate * (ngx_time() - c->start_sec + 1)
                 - (c->sent - r->limit_rate_after);
 
         if (limit <= 0) {