patch proposal

[Maxim Dounin]

Hello!

On Fri, Apr 08, 2016 at 11:19:17AM +0300, Vladislav Shabanov wrote:

> Если так письмо на русском не примете, я его, конечно, переведу 
> на английский, но хотелось бы чуть-чуть времени сэкономить :)

This list language is English.  If you want to use Russian -
please use nginx-ru@ mailinig list.

[...]

> Можно поставить nodelay, но тогда слишком активные боты не будут 
> получать задержек и ситуация будет доходить до выдачи им ошибки 
> 503.
> 
> Таким образом, мне нужно, чтобы при незначительном превышении 
> частоты обращений nodelay не было, а при многократном превышении 
> он, наоборот, был и помогал не доводить поисковых ботов до 503 
> ошибки.

So, the idea is that there should be two states:

1. Requests are passed without being delayed till certain number of 
   requests are accumulated in the bucket.

2. Then further requests are delayed till burst= is reached.

So, basically, there are two burst limits now: delay limit 
(nodelay=, defaults to 0, set to a large value if no argument 
specified) and reject limit (burst=).

Correct?

Something like this was discussed couple of times already, though 
I tend to like the approach which just changes "nodelay" into an 
additional limit.

The patch itself certainly needs more work to be committable, 
below is an alternative version.  (Completely untested though.)

I also think that we may consider using some reasonable value by 
default (something like burst/2?).  The delay mode as used by 
default seems to be misused very often.

# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1460135272 -10800
#      Fri Apr 08 20:07:52 2016 +0300
# Node ID 3cac97f32d5ee71a5b712d1d88ce97966b72a374
# Parent  2e722c177a5b35e4d88229e681b605542dfb1452
Limit req: "nodelay=" parameter.

This parameter specifies an additional "soft" burst limit at which requests
become delayed (but not yet rejected as it happens if "burst=" limit is
exceeded).  Defaults to 0, i.e., all excess requests are delayed.

Inspired by Vladislav Shabanov,
http://mailman.nginx.org/pipermail/nginx-devel/2016-April/008126.html.

diff --git a/src/http/modules/ngx_http_limit_req_module.c b/src/http/modules/ngx_http_limit_req_module.c
--- a/src/http/modules/ngx_http_limit_req_module.c
+++ b/src/http/modules/ngx_http_limit_req_module.c
@@ -44,7 +44,7 @@ typedef struct {
     ngx_shm_zone_t              *shm_zone;
     /* integer value, 1 corresponds to 0.001 r/s */
     ngx_uint_t                   burst;
-    ngx_uint_t                   nodelay; /* unsigned  nodelay:1 */
+    ngx_uint_t                   nodelay;
 } ngx_http_limit_req_limit_t;
 
 
@@ -491,7 +491,7 @@ ngx_http_limit_req_account(ngx_http_limi
 
     excess = *ep;
 
-    if (excess == 0 || (*limit)->nodelay) {
+    if ((ngx_uint_t) excess <= (*limit)->nodelay) {
         max_delay = 0;
 
     } else {
@@ -528,7 +528,7 @@ ngx_http_limit_req_account(ngx_http_limi
 
         ctx->node = NULL;
 
-        if (limits[n].nodelay) {
+        if ((ngx_uint_t) excess <= limits[n].nodelay) {
             continue;
         }
 
@@ -862,9 +862,9 @@ ngx_http_limit_req(ngx_conf_t *cf, ngx_c
 {
     ngx_http_limit_req_conf_t  *lrcf = conf;
 
-    ngx_int_t                    burst;
+    ngx_int_t                    burst, nodelay;
     ngx_str_t                   *value, s;
-    ngx_uint_t                   i, nodelay;
+    ngx_uint_t                   i;
     ngx_shm_zone_t              *shm_zone;
     ngx_http_limit_req_limit_t  *limit, *limits;
 
@@ -895,7 +895,7 @@ ngx_http_limit_req(ngx_conf_t *cf, ngx_c
             burst = ngx_atoi(value[i].data + 6, value[i].len - 6);
             if (burst <= 0) {
                 ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
-                                   "invalid burst rate \"%V\"", &value[i]);
+                                   "invalid burst value \"%V\"", &value[i]);
                 return NGX_CONF_ERROR;
             }
 
@@ -903,7 +903,19 @@ ngx_http_limit_req(ngx_conf_t *cf, ngx_c
         }
 
         if (ngx_strcmp(value[i].data, "nodelay") == 0) {
-            nodelay = 1;
+            nodelay = NGX_MAX_INT_T_VALUE / 1000;
+            continue;
+        }
+
+        if (ngx_strncmp(value[i].data, "nodelay=", 8) == 0) {
+
+            nodelay = ngx_atoi(value[i].data + 6, value[i].len - 6);
+            if (nodelay <= 0) {
+                ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
+                                   "invalid nodelay value \"%V\"", &value[i]);
+                return NGX_CONF_ERROR;
+            }
+
             continue;
         }
 
@@ -943,7 +955,7 @@ ngx_http_limit_req(ngx_conf_t *cf, ngx_c
 
     limit->shm_zone = shm_zone;
     limit->burst = burst * 1000;
-    limit->nodelay = nodelay;
+    limit->nodelay = nodelay * 1000;
 
     return NGX_CONF_OK;
 }


-- 
Maxim Dounin
http://nginx.org/