From fdf9008ab026915138e47bf05325c19da2b2b6fb Mon Sep 17 00:00:00 2001 From: Thibault Charbonnier Date: Tue, 30 Jan 2024 17:30:53 -0800 Subject: [PATCH] feat(mlcache) new 'get()' option: resty_lock_opts --- README.md | 3 ++ lib/resty/mlcache.lua | 31 ++++++++++---- t/02-get.t | 98 +++++++++++++++++++++++++++++++++++++------ 3 files changed, 111 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 785ceb9e..16587c20 100644 --- a/README.md +++ b/README.md @@ -410,6 +410,9 @@ options: having to repeat such transformations on every request, such as creating tables, cdata objects, loading new Lua code, etc... **Default:** inherited from the instance. +- `resty_lock_opts`: _optional_ table. If specified, override the instance + `resty_lock_opts` for the current `get()` lookup. + **Default:** inherited from the instance. The third argument `callback` is optional. If provided, it must be a function whose signature and return values are documented in the following example: diff --git a/lib/resty/mlcache.lua b/lib/resty/mlcache.lua index 136a9efc..a5f3f734 100644 --- a/lib/resty/mlcache.lua +++ b/lib/resty/mlcache.lua @@ -616,6 +616,7 @@ local function check_opts(self, opts) local resurrect_ttl local l1_serializer local shm_set_tries + local resty_lock_opts if opts ~= nil then if type(opts) ~= "table" then @@ -670,6 +671,13 @@ local function check_opts(self, opts) error("opts.shm_set_tries must be >= 1", 3) end end + + resty_lock_opts = opts.resty_lock_opts + if resty_lock_opts ~= nil then + if type(resty_lock_opts) ~= "table" then + error("opts.resty_lock_opts must be a table", 3) + end + end end if not ttl then @@ -692,7 +700,12 @@ local function check_opts(self, opts) shm_set_tries = self.shm_set_tries end - return ttl, neg_ttl, resurrect_ttl, l1_serializer, shm_set_tries + if not resty_lock_opts then + resty_lock_opts = self.resty_lock_opts + end + + return ttl, neg_ttl, resurrect_ttl, l1_serializer, shm_set_tries, + resty_lock_opts end @@ -707,9 +720,9 @@ end local function run_callback(self, key, shm_key, data, ttl, neg_ttl, - went_stale, l1_serializer, resurrect_ttl, shm_set_tries, cb, ...) + went_stale, l1_serializer, resurrect_ttl, shm_set_tries, rlock_opts, cb, ...) - local lock, err = resty_lock:new(self.shm_locks, self.resty_lock_opts) + local lock, err = resty_lock:new(self.shm_locks, rlock_opts) if not lock then return nil, "could not create lock: " .. err end @@ -877,8 +890,8 @@ function _M:get(key, opts, cb, ...) -- opts validation - local ttl, neg_ttl, resurrect_ttl, l1_serializer, shm_set_tries = - check_opts(self, opts) + local ttl, neg_ttl, resurrect_ttl, l1_serializer, shm_set_tries, + rlock_opts = check_opts(self, opts) local err, went_stale, is_stale data, err, went_stale, is_stale = get_shm_set_lru(self, key, namespaced_key, @@ -906,7 +919,7 @@ function _M:get(key, opts, cb, ...) return run_callback(self, key, namespaced_key, data, ttl, neg_ttl, went_stale, l1_serializer, resurrect_ttl, - shm_set_tries, cb, ...) + shm_set_tries, rlock_opts, cb, ...) end @@ -922,6 +935,7 @@ local function run_thread(self, ops, from, to) ctx.l1_serializer, ctx.resurrect_ttl, ctx.shm_set_tries, + ctx.rlock_opts, ctx.cb, ctx.arg) end end @@ -1042,8 +1056,8 @@ function _M:get_bulk(bulk, opts) res[res_idx + 2] = 1 else - local pok, ttl, neg_ttl, resurrect_ttl, l1_serializer, shm_set_tries - = pcall(check_opts, self, b_opts) + local pok, ttl, neg_ttl, resurrect_ttl, l1_serializer, + shm_set_tries, rlock_opts = pcall(check_opts, self, b_opts) if not pok then -- strip the stacktrace local err = ttl:match("mlcache%.lua:%d+:%s(.*)") @@ -1096,6 +1110,7 @@ function _M:get_bulk(bulk, opts) ctx.l1_serializer = l1_serializer ctx.resurrect_ttl = resurrect_ttl ctx.shm_set_tries = shm_set_tries + ctx.rlock_opts = rlock_opts ctx.data = data ctx.err = nil ctx.hit_lvl = nil diff --git a/t/02-get.t b/t/02-get.t index c3d73546..9cc9f17b 100644 --- a/t/02-get.t +++ b/t/02-get.t @@ -1766,7 +1766,35 @@ in positive callback -=== TEST 42: get() passes 'resty_lock_opts' for L3 calls +=== TEST 42: get() errors on invalid opts.resty_lock_opts +--- config + location /t { + content_by_lua_block { + local mlcache = require "resty.mlcache" + + local cache, err = mlcache.new("my_mlcache", "cache_shm") + if not cache then + ngx.log(ngx.ERR, err) + return + end + + local ok, err = pcall(cache.get, cache, "key", { + resty_lock_opts = "true" + }) + if not ok then + ngx.say(err) + end + } + } +--- response_body +opts.resty_lock_opts must be a table +--- no_error_log +[error] +[crit] + + + +=== TEST 43: get() passes 'resty_lock_opts' for L3 calls --- config location /t { content_by_lua_block { @@ -1807,7 +1835,51 @@ was given 'opts.resty_lock_opts': true -=== TEST 43: get() errors on lock timeout +=== TEST 44: get() uses 'opts.resty_lock_opts' if specified +--- config + location /t { + content_by_lua_block { + local resty_lock = require "resty.lock" + local mlcache = require "resty.mlcache" + + local opts = { timeout = 1 } + local resty_lock_opts = { timeout = 5 } + + do + local orig_resty_lock_new = resty_lock.new + resty_lock.new = function(_, dict_name, opts, ...) + ngx.say("was given 'opts.resty_lock_opts': ", opts == resty_lock_opts) + + return orig_resty_lock_new(_, dict_name, opts, ...) + end + end + + local cache, err = mlcache.new("my_mlcache", "cache_shm", { + resty_lock_opts = opts, + }) + if not cache then + ngx.log(ngx.ERR, err) + return + end + + local data, err = cache:get("key", { + resty_lock_opts = resty_lock_opts, + }, function() return nil end) + if err then + ngx.log(ngx.ERR, err) + return + end + } + } +--- response_body +was given 'opts.resty_lock_opts': true +--- no_error_log +[error] +[crit] + + + +=== TEST 45: get() errors on lock timeout --- config location /t { access_by_lua_block { @@ -1885,7 +1957,7 @@ hit_lvl: 1 -=== TEST 44: get() returns data even if failed to set in shm +=== TEST 46: get() returns data even if failed to set in shm --- config location /t { content_by_lua_block { @@ -1934,7 +2006,7 @@ qr/\[warn\] .*? could not write to lua_shared_dict 'cache_shm' after 3 tries \(n -=== TEST 45: get() errors on invalid opts.shm_set_tries +=== TEST 47: get() errors on invalid opts.shm_set_tries --- config location /t { content_by_lua_block { @@ -1972,7 +2044,7 @@ opts.shm_set_tries must be >= 1 -=== TEST 46: get() with default shm_set_tries to LRU evict items when a large value is being cached +=== TEST 48: get() with default shm_set_tries to LRU evict items when a large value is being cached --- config location /t { content_by_lua_block { @@ -2040,7 +2112,7 @@ callback was called: 1 times -=== TEST 47: get() respects instance opts.shm_set_tries to LRU evict items when a large value is being cached +=== TEST 49: get() respects instance opts.shm_set_tries to LRU evict items when a large value is being cached --- config location /t { content_by_lua_block { @@ -2110,7 +2182,7 @@ callback was called: 1 times -=== TEST 48: get() accepts opts.shm_set_tries to LRU evict items when a large value is being cached +=== TEST 50: get() accepts opts.shm_set_tries to LRU evict items when a large value is being cached --- config location /t { content_by_lua_block { @@ -2180,7 +2252,7 @@ callback was called: 1 times -=== TEST 49: get() caches data in L1 LRU even if failed to set in shm +=== TEST 51: get() caches data in L1 LRU even if failed to set in shm --- config location /t { content_by_lua_block { @@ -2242,7 +2314,7 @@ is stale: true -=== TEST 50: get() does not cache value in LRU indefinitely when retrieved from shm on last ms (see GH PR #58) +=== TEST 52: get() does not cache value in LRU indefinitely when retrieved from shm on last ms (see GH PR #58) --- config location /t { content_by_lua_block { @@ -2305,7 +2377,7 @@ is stale: true -=== TEST 51: get() bypass cache for negative callback TTL +=== TEST 53: get() bypass cache for negative callback TTL --- config location /t { content_by_lua_block { @@ -2370,7 +2442,7 @@ in negative callback -=== TEST 52: get() nil callback returns positive cached items from L1/L2 +=== TEST 54: get() nil callback returns positive cached items from L1/L2 --- config location /t { content_by_lua_block { @@ -2452,7 +2524,7 @@ hit_lvl: 1 -=== TEST 53: get() nil callback returns negative cached items from L1/L2 +=== TEST 55: get() nil callback returns negative cached items from L1/L2 --- config location /t { content_by_lua_block { @@ -2534,7 +2606,7 @@ hit_lvl: 1 -=== TEST 54: get() JITs on misses without a callback +=== TEST 56: get() JITs on misses without a callback --- config location /t { content_by_lua_block {