Le mer. 30 nov. 2022 à 04:42, Maxim Dounin <mdounin@mdounin.ru> a écrit :
>
> Hello!
>
> On Mon, Nov 28, 2022 at 06:40:33AM +0100, Jérôme Loyet wrote:
>
> > Hello,
> >
> > I'm using rate limiting within the stream module. While it works great
> > for long connections it does not work on request smaller than the rate
> > limite size for 1 second. I set up a 1gbps rate limit (limit_rate
> > 125m) and request smaller than 125M or not limited. This is a normal
> > behavioir as the rate limiting is done with a second precision. This
> > patch change second precision to millisecond precision. From my first
> > tests (still on going) it seems to works better.
> >
> > What guys do you think about this patch ?
>
> A while ago a similar patch was considered for both stream and
> http, though there is an open question on how to handle
> long-running requests on 32-bit platforms (where ngx_msec_t will
> overflow after ~48 days). With the naive approach, which is also
> seen in your patch, all traffic on a such connection is likely to
> completely stop after the overflow, which does not seem to be a
> good outcome.

Thanks you maxime for your feedbacks.

here is another proposal. To address this issue, let's use the the
millisecond for the first 10 seconds and switch back to seconds for
the rest of the transfer.

would it be acceptable ?

Regards
++ Jerome

# HG changeset patch
# User Jerome Loyet <jerome@loyet.net>
# Date 1676019332 -3600
# Fri Feb 10 09:55:32 2023 +0100
# Node ID f381512e0f116b0d63751a48996cacd3963954ca
# Parent cffaf3f2eec8fd33605c2a37814f5ffc30371989
switch to ms resolution for rate limiting

rate limiting use a second resolution which can be bypassed on
requests/response smaller than the configured rate for 1 second.

This patch switches to millisecond resolution to ensure
better precision of the rate limiting.

Millisecond resolution is only used the first 10 seconds
and switches back to seconds resolution after to prevent
overflowing the variable.

diff -r cffaf3f2eec8 -r f381512e0f11 src/event/ngx_event_pipe.c
--- a/src/event/ngx_event_pipe.c Thu Feb 02 23:38:48 2023 +0300
+++ b/src/event/ngx_event_pipe.c Fri Feb 10 09:55:32 2023 +0100
@@ -104,6 +104,7 @@
off_t limit;
ssize_t n, size;
ngx_int_t rc;
+ ngx_uint_t time_diff;
ngx_buf_t *b;
ngx_msec_t delay;
ngx_chain_t *chain, *cl, *ln;
@@ -203,8 +204,14 @@
break;
}

- limit = (off_t) p->limit_rate * (ngx_time() - p->start_sec + 1)
- - p->read_length;
+ // use millisecond resolution for the first 10 seconds
+ // then switch to second resolution to prevent
variable overflow
+ if (ngx_time() - u-> start_sec < 10) {
+ time_diff = (ngx_current_msec - u->start_msec + 1) / 1000;
+ } else {
+ time_diff = ngx_time() - u->start_sec + 1;
+ }
+ limit = (off_t) p->limit_rate * time_diff - p->read_length;

if (limit <= 0) {
p->upstream->read->delayed = 1;
diff -r cffaf3f2eec8 -r f381512e0f11 src/event/ngx_event_pipe.h
--- a/src/event/ngx_event_pipe.h Thu Feb 02 23:38:48 2023 +0300
+++ b/src/event/ngx_event_pipe.h Fri Feb 10 09:55:32 2023 +0100
@@ -92,6 +92,7 @@

size_t limit_rate;
time_t start_sec;
+ ngx_msec_t start_msec;

ngx_temp_file_t *temp_file;

diff -r cffaf3f2eec8 -r f381512e0f11 src/http/ngx_http_upstream.c
--- a/src/http/ngx_http_upstream.c Thu Feb 02 23:38:48 2023 +0300
+++ b/src/http/ngx_http_upstream.c Fri Feb 10 09:55:32 2023 +0100
@@ -3218,6 +3218,7 @@
p->log = c->log;
p->limit_rate = u->conf->limit_rate;
p->start_sec = ngx_time();
+ p->start_msec = ngx_current_msec;

p->cacheable = u->cacheable || u->store;

diff -r cffaf3f2eec8 -r f381512e0f11 src/stream/ngx_stream_proxy_module.c
--- a/src/stream/ngx_stream_proxy_module.c Thu Feb 02 23:38:48 2023 +0300
+++ b/src/stream/ngx_stream_proxy_module.c Fri Feb 10 09:55:32 2023 +0100
@@ -436,6 +436,7 @@

u->peer.type = c->type;
u->start_sec = ngx_time();
+ u->start_msec = ngx_current_msec;

c->write->handler = ngx_stream_proxy_downstream_handler;
c->read->handler = ngx_stream_proxy_downstream_handler;
@@ -1594,7 +1595,7 @@
ssize_t n;
ngx_buf_t *b;
ngx_int_t rc;
- ngx_uint_t flags, *packets;
+ ngx_uint_t flags, *packets, time_diff;
ngx_msec_t delay;
ngx_chain_t *cl, **ll, **out, **busy;
ngx_connection_t *c, *pc, *src, *dst;
@@ -1678,8 +1679,14 @@
if (size && src->read->ready && !src->read->delayed) {

if (limit_rate) {
- limit = (off_t) limit_rate * (ngx_time() - u->start_sec + 1)
- - *received;
+ // use millisecond resolution for the first 10 seconds
+ // then switch to second resolution to prevent
variable overflow
+ if (ngx_time() - u-> start_sec < 10) {
+ time_diff = (ngx_current_msec - u->start_msec + 1) / 1000;
+ } else {
+ time_diff = ngx_time() - u->start_sec + 1;
+ }
+ limit = (off_t) limit_rate * time_diff - *received;

if (limit <= 0) {
src->read->delayed = 1;
diff -r cffaf3f2eec8 -r f381512e0f11 src/stream/ngx_stream_upstream.h
--- a/src/stream/ngx_stream_upstream.h Thu Feb 02 23:38:48 2023 +0300
+++ b/src/stream/ngx_stream_upstream.h Fri Feb 10 09:55:32 2023 +0100
@@ -128,6 +128,7 @@

off_t received;
time_t start_sec;
+ ngx_msec_t start_msec;
ngx_uint_t requests;
ngx_uint_t responses;
ngx_msec_t start_time;
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel