Modules: improved shared dict eviction strategy.

Previously, when a slab allocation failed in evict mode, only 16
entries were evicted with a single retry.  This could still result
in SharedMemoryError when the freed slab slots did not match the
requested allocation size class, even though the zone had plenty
of evictable entries.

In practice, it might happen when the following conditions are met:
    - The shared zone is full
    - evict flag is enabled
    - key/value entries differ in size

The allocation now retries in a loop, evicting 16 entries at a time,
until the allocation succeeds or no more entries remain in the expire
tree.

After this change, allocation with evict enabled can only fail when:
    - the value is larger than the zone's usable space
    - the expire tree has no entries left to evict
    - zone metadata overhead leaves insufficient room

diff --git a/nginx/ngx_js_shared_dict.c b/nginx/ngx_js_shared_dict.c
index b4a09bb16f0c7d038727351164e9cf69ad24e22f..54af5028d00ab07c673a8d00b0b3183afbba0728 100644 (file)
--- a/nginx/ngx_js_shared_dict.c
+++ b/nginx/ngx_js_shared_dict.c
@@ -128,7 +128,7 @@ static ngx_int_t ngx_js_dict_copy_value_locked(njs_vm_t *vm,
     ngx_js_dict_t *dict, ngx_js_dict_node_t *node, njs_value_t *retval);
 
 static void ngx_js_dict_expire(ngx_js_dict_t *dict, ngx_msec_t now);
-static void ngx_js_dict_evict(ngx_js_dict_t *dict, ngx_int_t count);
+static ngx_uint_t ngx_js_dict_evict(ngx_js_dict_t *dict, ngx_uint_t count);
 
 static njs_int_t ngx_js_dict_shared_error_name(njs_vm_t *vm,
     njs_object_prop_t *prop, uint32_t unused, njs_value_t *value,
@@ -1371,12 +1371,24 @@ ngx_js_dict_alloc(ngx_js_dict_t *dict, size_t n)
 {
     void  *p;
 
-    dict->shpool->log_nomem = !dict->evict;
+    if (!dict->evict) {
+        return ngx_slab_alloc_locked(dict->shpool, n);
+    }
+
+    dict->shpool->log_nomem = 0;
     p = ngx_slab_alloc_locked(dict->shpool, n);
+
+    while (p == NULL) {
+        if (ngx_js_dict_evict(dict, 16) == 0) {
+            break;
+        }
+
+        p = ngx_slab_alloc_locked(dict->shpool, n);
+    }
+
     dict->shpool->log_nomem = 1;
 
-    if (p == NULL && dict->evict) {
-        ngx_js_dict_evict(dict, 16);
+    if (p == NULL) {
         p = ngx_slab_alloc_locked(dict->shpool, n);
     }
 
@@ -1784,9 +1796,10 @@ ngx_js_dict_expire(ngx_js_dict_t *dict, ngx_msec_t now)
 }
 
 
-static void
-ngx_js_dict_evict(ngx_js_dict_t *dict, ngx_int_t count)
+static ngx_uint_t
+ngx_js_dict_evict(ngx_js_dict_t *dict, ngx_uint_t count)
 {
+    ngx_uint_t           evicted;
     ngx_rbtree_t        *rbtree;
     ngx_rbtree_node_t   *rn, *next;
     ngx_js_dict_node_t  *node;
@@ -1794,15 +1807,17 @@ ngx_js_dict_evict(ngx_js_dict_t *dict, ngx_int_t count)
     rbtree = &dict->sh->rbtree_expire;
 
     if (rbtree->root == rbtree->sentinel) {
-        return;
+        return 0;
     }
 
+    evicted = 0;
+
     for (rn = ngx_rbtree_min(rbtree->root, rbtree->sentinel);
          rn != NULL;
          rn = next)
     {
         if (count-- == 0) {
-            return;
+            return evicted;
         }
 
         node = (ngx_js_dict_node_t *)
@@ -1815,7 +1830,11 @@ ngx_js_dict_evict(ngx_js_dict_t *dict, ngx_int_t count)
         ngx_rbtree_delete(&dict->sh->rbtree, (ngx_rbtree_node_t *) node);
 
         ngx_js_dict_node_free(dict, node);
+
+        evicted++;
     }
+
+    return evicted;
 }
 
 

diff --git a/nginx/t/js_shared_dict_evict.t b/nginx/t/js_shared_dict_evict.t
index 3501c2382ccd89f22f763df12b80e62e6ea68938..fe0b381ba9a8a5e9db2cb0035602bec4df6c1ade 100644 (file)
--- a/nginx/t/js_shared_dict_evict.t
+++ b/nginx/t/js_shared_dict_evict.t
@@ -51,6 +51,10 @@ http {
         location /no_self_evict {
             js_content test.no_self_evict;
         }
+
+        location /cross_slab_class {
+            js_content test.cross_slab_class;
+        }
     }
 }
 
@@ -110,12 +114,38 @@ $t->write_file('test.js', <<'EOF');
         r.return(200, 'FILLED:' + elems);
     }
 
-    export default { stress, no_self_evict };
+    function cross_slab_class(r) {
+        var dict = ngx.shared.stress;
+        var v = 'x'.repeat(16);
+
+        /* Calibrate: find how many tiny entries fit. */
+        dict.clear();
+        dict.set('probe', v);
+
+        var elems = 0;
+        while (dict.has('probe')) {
+            dict.set('s_' + elems++, v);
+        }
+
+        /* Rebuild at exact capacity with tiny entries. */
+        dict.clear();
+        for (var i = 0; i < elems; i++) {
+            dict.set('s_' + i, v);
+        }
+
+        /* Check that the zone can evict enough entries to fit a big one. */
+
+        dict.set('big', 'y'.repeat(2048));
+
+        r.return(200, 'FILLED:' + elems);
+    }
+
+    export default { stress, no_self_evict, cross_slab_class };
 EOF
 
 $t->try_run('no js_shared_dict_zone');
 
-$t->plan(6);
+$t->plan(7);
 
 ###############################################################################
 
@@ -131,4 +161,7 @@ like($update_resp, qr/FILLED:/, 'evict update: zone filled');
 unlike($t->read_file('error.log'), qr/is already free/,
        'evict update: no double-free in error log');
 
+like(http_get('/cross_slab_class'), qr/FILLED:/,
+       'evict cross slab class: large alloc after small entries');
+
 ###############################################################################