Hello, Maxim!
2016 year patch seems pretty same to me :) Interesting that I also made my patch at 2016, but we have started to use it in production a while ago (when upgraded to nginx 1.15.4 where you've fixed limit_req issue)
I think calculating excess above nodelay should be more expected behavior. Think about setting large burst and large nodelay, for ex. burst=10000 nodelay=9999. Even at relatively fast limit rate (10r/s) this will therefore lead to very large delay which I think is not expected nor by user neither by server administrator. I think it is more intuitive to think of it as you give nodelay requests for for free and additional requests with limited speed, instead of you give nodelay requests as a credit and then force client to wait all that "credited" time when he wants to make additional requests.
Also I think it doesn't have sense to allow nodelay to be larger than burst because in any case requests that are more then burst will be rejected. May be it's better to just warn user and implicitly reduce nodelay value to burst value in that case instead of error.
So I would suggest to use behavior that calculates delay based on excess above nodelay.
If you have any suggestions on fixing configuration parsing behavior or any other suggestions on my code I'm ready to make fixes.
Thank you!
Peter Shchuhckin
29.10.2018, 15:58, "Maxim Dounin" <mdou...@mdounin.ru>:
Hello!
On Sun, Oct 28, 2018 at 05:58:45PM +0300, Peter Shchuchkin wrote:
# HG changeset patch
# User Peter Shchuchkin <pet...@yandex.ru>
# Date 1540737213 -10800
# Sun Oct 28 17:33:33 2018 +0300
# Node ID 70c0d476999d9b893c644606624134248ac7abad
# Parent 874d47ac871a4b62fbe0ff5d469a8ad7bc5a4160
Allow using nodelay=N semantics in limit_req configuration
This allows to use reasonably low limits while not forcing delay on normal
users.
In addition to standard "burst=A nodelay" form now the following form of
limit_req may be used:
burst=A nodelay=B, where B must be 0 <= B <= burst
burst=A nodelay=0 means the same as just "burst=A"
burst=A nodelay=A means the same as "burst=A nodelay"
burst=A nodelay=B means the first B requests matching limit_zone variable will
not be delayed and next requests will be delayed. The delay is calculated
against excess over B thus B+1 request will have effective excess=1.
When using limit_req with nodelay the responsibility of limiting requests speed
is on the client.
If client don't want or can't correctly limit its speed it will get 503 errors
and you will get numerous messages in error and access logs.
When using limit_req without nodelay, then every request that comes faster then
expected speed will be delayed. This is not always convenient. Sometimes you
want to allow normal client to make a bunch of requests as fast as possible
while still having configured limit on request speed.
Using this new semantics you can get the best from two worlds. Specifying
burst=A nodelay=B you allow clients to make B requests without any delay (and
without warnings in error log). If B requests are exceeded by client then
further requests are delayed, effectively limiting client rps to desired limit
without returning 503 errors. Thus one can ensure maximum speed for clients
with expected usage profile and limit all other clients to certain speed
without errors.--
[...]
I've posted a patch for this a while ago, see here:
http://mailman.nginx.org/pipermail/nginx-devel/2016-April/008136.html
Please take a look if it works for you, and/or you have any
comments.
The most noticeable difference I see is how delay is calculated, and
probably your variant with only counting excess above the nodelay
level is more logical.
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
_______________________________________________ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
