fix(mlcache) do not error out on 'no memory'

README.md

@@ -232,6 +232,13 @@ holding the desired options for this instance. The possible options are:
   cache.  It can thus avoid your application from having to repeat such
   transformation upon every cache hit, such as creating tables, cdata objects,
   functions, etc...
+- `shm_set_tries`: the number of tries for the lua_shared_dict `set()`
+  operation. When the lua_shared_dict is full, it attempts to free up to 30
+  items from its queue. When the value being set is much larger than the freed
+  space, this option allows mlcache to retry the operation (and free more slots)
+  until the maximum number of tries is reached or enough memory was freed for
+  the value to fit.
+  **Default**: `3`.
 - `ipc_shm`: _optional_ string. If you wish to use [set()](#set),
   [delete()](#delete), or [purge()](#purge), you must provide an IPC
   (Inter-process communication) mechanism for workers to invalidate their L1

lib/resty/mlcache.lua

@@ -13,15 +13,18 @@ local find         = string.find
 local type         = type
 local pcall        = pcall
 local error        = error
-local shared       = ngx.shared
 local tostring     = tostring
 local tonumber     = tonumber
 local setmetatable = setmetatable
+local shared       = ngx.shared
+local ngx_log      = ngx.log
+local WARN         = ngx.WARN
 
 
 local LOCK_KEY_PREFIX         = "lua-resty-mlcache:lock:"
 local CACHE_MISS_SENTINEL_LRU = {}
 local LRU_INSTANCES           = {}
+local SHM_SET_DEFAULT_TRIES   = 3
 
 
 local c_str_type    = ffi.typeof("char *")
@@ -252,6 +255,16 @@ function _M.new(name, shm, opts)
         then
             error("opts.l1_serializer must be a function", 2)
         end
+
+        if opts.shm_set_tries ~= nil then
+            if type(opts.shm_set_tries) ~= "number" then
+                error("opts.shm_set_tries must be a number", 2)
+            end
+
+            if opts.shm_set_tries < 1 then
+                error("opts.shm_set_tries must be >= 1", 2)
+            end
+        end
     else
         opts = {}
     end
@@ -270,6 +283,7 @@ function _M.new(name, shm, opts)
         lru_size        = opts.lru_size or 100,
         resty_lock_opts = opts.resty_lock_opts,
         l1_serializer   = opts.l1_serializer,
+        shm_set_tries   = opts.shm_set_tries or SHM_SET_DEFAULT_TRIES,
     }
 
     if opts.ipc_shm or opts.ipc then
@@ -377,17 +391,53 @@ local function set_lru(self, key, value, ttl, neg_ttl, l1_serializer)
 end
 
 
-local function set_shm(self, shm_key, value, ttl, neg_ttl)
+local function shm_set_retries(self, key, val, ttl, max_tries)
+    -- we will call `set()` N times to work around potential shm fragmentation.
+    -- when the shm is full, it will only evict about 30 to 90 items (via
+    -- LRU), which could lead to a situation where `set()` still does not
+    -- have enough memory to store the cached value, in which case we
+    -- try again to try to trigger more LRU evictions.
+
+    local tries = 0
+    local ok, err
+
+    while tries < max_tries do
+        tries = tries + 1
+
+        ok, err = self.dict:set(key, val, ttl)
+        if ok or err and err ~= "no memory" then
+            break
+        end
+    end
+
+    if not ok then
+        if err ~= "no memory" then
+            return nil, "could not write to lua_shared_dict '" .. self.shm
+                        .. "': " .. err
+        end
+
+        ngx_log(WARN, "could not write to lua_shared_dict '",
+                      self.shm, "' after ", tries, " tries (no memory), ",
+                      "it is either fragmented or cannot allocate more ",
+                      "memory, consider increasing 'opts.shm_set_tries'")
+    end
+
+    return true
+end
+
+
+local function set_shm(self, shm_key, value, ttl, neg_ttl, shm_set_tries)
     local at = now()
 
     if value == nil then
         local shm_nil = marshallers.shm_nil(at, neg_ttl)
 
         -- we need to cache that this was a miss, and ensure cache hit for a
         -- nil value
-        local ok, err = self.dict:set(shm_key, shm_nil, neg_ttl)
+        local ok, err = shm_set_retries(self, shm_key, shm_nil, neg_ttl,
+                                        shm_set_tries)
         if not ok then
-            return nil, "could not write to lua_shared_dict: " .. err
+            return nil, err
         end
 
         return true
@@ -412,9 +462,10 @@ local function set_shm(self, shm_key, value, ttl, neg_ttl)
 
     -- cache value in shm for currently-locked workers
 
-    local ok, err = self.dict:set(shm_key, shm_marshalled, ttl)
+    local ok, err = shm_set_retries(self, shm_key, shm_marshalled, ttl,
+                                    shm_set_tries)
     if not ok then
-        return nil, "could not write to lua_shared_dict: " .. err
+        return nil, err
     end
 
     return true
@@ -462,6 +513,7 @@ local function check_opts(self, opts)
     local ttl
     local neg_ttl
     local l1_serializer
+    local shm_set_tries
 
     if opts ~= nil then
         if type(opts) ~= "table" then
@@ -494,6 +546,17 @@ local function check_opts(self, opts)
         if l1_serializer ~= nil and type(l1_serializer) ~= "function" then
            error("opts.l1_serializer must be a function", 3)
         end
+
+        shm_set_tries = opts.shm_set_tries
+        if shm_set_tries ~= nil then
+            if type(shm_set_tries) ~= "number" then
+                error("opts.shm_set_tries must be a number", 3)
+            end
+
+            if shm_set_tries < 1 then
+                error("opts.shm_set_tries must be >= 1", 3)
+            end
+        end
     end
 
     if not ttl then
@@ -508,7 +571,11 @@ local function check_opts(self, opts)
         l1_serializer = self.l1_serializer
     end
 
-    return ttl, neg_ttl, l1_serializer
+    if not shm_set_tries then
+        shm_set_tries = self.shm_set_tries
+    end
+
+    return ttl, neg_ttl, l1_serializer, shm_set_tries
 end
 
 
@@ -551,7 +618,7 @@ function _M:get(key, opts, cb, ...)
 
     -- opts validation
 
-    local ttl, neg_ttl, l1_serializer = check_opts(self, opts)
+    local ttl, neg_ttl, l1_serializer, shm_set_tries = check_opts(self, opts)
 
     local err
     data, err = get_shm_set_lru(self, key, namespaced_key, l1_serializer)
@@ -622,7 +689,8 @@ function _M:get(key, opts, cb, ...)
 
     -- set shm cache level
 
-    local ok, err = set_shm(self, namespaced_key, data, ttl, neg_ttl)
+    local ok, err = set_shm(self, namespaced_key, data, ttl, neg_ttl,
+                            shm_set_tries)
     if not ok then
         return unlock_and_ret(lock, nil, err)
     end
@@ -693,12 +761,14 @@ function _M:set(key, opts, value)
         -- restrict this key to the current namespace, so we isolate this
         -- mlcache instance from potential other instances using the same
         -- shm
-        local ttl, neg_ttl, l1_serializer = check_opts(self, opts)
+        local ttl, neg_ttl, l1_serializer, shm_set_tries = check_opts(self,
+                                                                      opts)
         local namespaced_key = self.name .. key
 
         set_lru(self, key, value, ttl, neg_ttl, l1_serializer)
 
-        local ok, err = set_shm(self, namespaced_key, value, ttl, neg_ttl)
+        local ok, err = set_shm(self, namespaced_key, value, ttl, neg_ttl,
+                                shm_set_tries)
         if not ok then
             return nil, err
         end

t/01-new.t

@@ -472,7 +472,41 @@ opts.resty_lock_opts must be a table
 
 
 
-=== TEST 18: new() creates an mlcache object with default attributes
+=== TEST 18: new() validates opts.shm_set_tries
+--- http_config eval: $::HttpConfig
+--- config
+    location = /t {
+        content_by_lua_block {
+            local mlcache = require "resty.mlcache"
+
+            local values = {
+                false,
+                -1,
+                0,
+            }
+
+            for _, v in ipairs(values) do
+                local ok, err = pcall(mlcache.new, "name", "cache_shm", {
+                    shm_set_tries = v,
+                })
+                if not ok then
+                    ngx.say(err)
+                end
+            end
+        }
+    }
+--- request
+GET /t
+--- response_body
+opts.shm_set_tries must be a number
+opts.shm_set_tries must be >= 1
+opts.shm_set_tries must be >= 1
+--- no_error_log
+[error]
+
+
+
+=== TEST 19: new() creates an mlcache object with default attributes
 --- http_config eval: $::HttpConfig
 --- config
     location = /t {
@@ -500,7 +534,7 @@ number
 
 
 
-=== TEST 19: new() accepts user-provided LRU instances via opts.lru
+=== TEST 20: new() accepts user-provided LRU instances via opts.lru
 --- http_config eval: $::HttpConfig
 --- config
     location = /t {