Skip to content

Commit

Permalink
feat(mlcache) implement negative TTL for misses
Browse files Browse the repository at this point in the history
  • Loading branch information
@thibaultcha
thibaultcha committed Apr 22, 2017
1 parent 657ca9a commit efe4b70
Show file tree
Hide file tree
Showing 3 changed files with 139 additions and 52 deletions.
83 changes: 57 additions & 26 deletions lib/resty/mlcache.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,30 @@ local _M = {}
local mt = { __index = _M }


function _M.new(shm, lru_size, ttl)
function _M.new(shm, opts)
if type(shm) ~= "string" then
return error("shm must be a string", 2)
return error("shm must be a string")
end

if type(lru_size) ~= "number" then
return error("lru_size must be a number", 2)
end
if opts then
if type(opts) ~= "table" then
return error("opts must be a table")
end

This comment has been minimized.

Sign in to view

Copy link
@Tieske

Tieske Jun 7, 2017 •
edited
Loading

Contributor

this style is very confusing, on first look it looks like it does return a nil + error. If you want to throw an error, then replace the return error(...) calls with an assert, that makes the intent a lot clearer imo.

There are lots of those.

if opts.lru_size and type(opts.lru_size) ~= "number" then
return error("opts.lru_size must be a number")
end

if ttl and type(ttl) ~= "number" then
return error("ttl must be a number", 2)
if opts.ttl and type(opts.ttl) ~= "number" then
return error("opts.ttl must be a number")
end

if opts.neg_ttl and type(opts.neg_ttl) ~= "number" then
return error("opts.neg_ttl must be a number")
end

else
opts = {}
end

local dict = shared[shm]
Expand All @@ -41,10 +54,11 @@ function _M.new(shm, lru_size, ttl)
end

local ml_cache = {
lru = lrucache.new(lru_size),
lru = lrucache.new(opts.lru_size or 100),
dict = dict,
shm = shm,
ttl = ttl or 30,
ttl = opts.ttl or 30,
neg_ttl = opts.neg_ttl or 5
}

return setmetatable(ml_cache, mt)
Expand All @@ -58,7 +72,7 @@ local function set_lru(self, key, value, ttl)
end


local function shmlru_get(self, key, ttl)
local function shmlru_get(self, key, ttl, neg_ttl)
local v, err = self.dict:get(key)
if err then
return nil, "could not read from lua_shared_dict: " .. err
Expand All @@ -70,7 +84,7 @@ local function shmlru_get(self, key, ttl)
end

if v == CACHE_MISS_SENTINEL_SHM then
return set_lru(self, key, CACHE_MISS_SENTINEL_LRU, ttl)
return set_lru(self, key, CACHE_MISS_SENTINEL_LRU, neg_ttl)
end

-- maybe need to decode if encoded
Expand All @@ -95,18 +109,18 @@ local function shmlru_get(self, key, ttl)
end


local function shmlru_set(self, key, value, ttl)
local function shmlru_set(self, key, value, ttl, neg_ttl)
if value == nil then
-- we need to cache that this was a miss, and ensure cache hit for a
-- nil value
local ok, err = self.dict:set(key, CACHE_MISS_SENTINEL_SHM, ttl)
local ok, err = self.dict:set(key, CACHE_MISS_SENTINEL_SHM, neg_ttl)
if not ok then
return nil, "could not write to lua_shared_dict: " .. err
end

-- set our own worker's LRU cache

self.lru:set(key, CACHE_MISS_SENTINEL_LRU, ttl)
self.lru:set(key, CACHE_MISS_SENTINEL_LRU, neg_ttl)

return nil
end
Expand Down Expand Up @@ -158,26 +172,45 @@ end

function _M:get(key, opts, cb, ...)
if type(key) ~= "string" then
return error("key must be a string", 2)
return error("key must be a string")
end

if type(cb) ~= "function" then
return error("callback must be a function", 2)
return error("callback must be a function")
end

-- opts validation

local ttl
local neg_ttl

if opts then
if type(opts) ~= "table" then
return error("opts must be a table", 2)
return error("opts must be a table")
end

if type(opts.ttl) ~= "number" then
return error("opts.ttl must be a number", 2)
if opts.ttl and type(opts.ttl) ~= "number" then
return error("opts.ttl must be a number")
end

else
opts = {}
if opts.neg_ttl and type(opts.neg_ttl) ~= "number" then
return error("opts.neg_ttl must be a number")
end

ttl = opts.ttl
neg_ttl = opts.neg_ttl
end

if not ttl then
ttl = self.ttl
end

if not neg_ttl then
neg_ttl = self.neg_ttl
end

-- worker LRU cache retrieval

local data = self.lru:get(key)
if data == CACHE_MISS_SENTINEL_LRU then
return nil
Expand All @@ -189,10 +222,8 @@ function _M:get(key, opts, cb, ...)

-- not in worker's LRU cache, need shm lookup

local ttl = opts.ttl or self.ttl

local err
data, err = shmlru_get(self, key, ttl)
data, err = shmlru_get(self, key, ttl, neg_ttl)
if err then
return nil, err
end
Expand Down Expand Up @@ -220,7 +251,7 @@ function _M:get(key, opts, cb, ...)

-- check for another worker's success at running the callback

data = shmlru_get(self, key, ttl)
data = shmlru_get(self, key, ttl, neg_ttl)
if data then
return unlock_and_ret(lock, data)
end
Expand All @@ -232,7 +263,7 @@ function _M:get(key, opts, cb, ...)
return unlock_and_ret(lock, nil, "callback threw an error: " .. err)
end

local value, err = shmlru_set(self, key, err, ttl)
local value, err = shmlru_set(self, key, err, ttl, neg_ttl)
if err then
return unlock_and_ret(lock, nil, err)
end
Expand Down
68 changes: 60 additions & 8 deletions t/01-new.t
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,14 @@ shm must be a string



=== TEST 2: new() validates lru_size
=== TEST 2: new() validates opts
--- http_config eval: $::HttpConfig
--- config
location = /t {
content_by_lua_block {
local mlcache = require "resty.mlcache"
local ok, err = pcall(mlcache.new, "")
local ok, err = pcall(mlcache.new, "", "foo")
if not ok then
ngx.log(ngx.ERR, err)
end
Expand All @@ -58,7 +58,7 @@ GET /t
--- response_body

--- error_log
lru_size must be a number
opts must be a table



Expand All @@ -69,7 +69,7 @@ lru_size must be a number
content_by_lua_block {
local mlcache = require "resty.mlcache"
local cache, err = mlcache.new("foo", 200)
local cache, err = mlcache.new("foo")
if not cache then
ngx.log(ngx.ERR, err)
end
Expand All @@ -84,14 +84,40 @@ no such lua_shared_dict: foo



=== TEST 4: new() validates ttl
=== TEST 4: new() validates lru_size
--- http_config eval: $::HttpConfig
--- config
location = /t {
content_by_lua_block {
local mlcache = require "resty.mlcache"
local ok, err = pcall(mlcache.new, "cache", {
lru_size = "",
})
if not ok then
ngx.log(ngx.ERR, err)
end
}
}
--- request
GET /t
--- response_body

--- error_log
lru_size must be a number



=== TEST 5: new() validates ttl
--- http_config eval: $::HttpConfig
--- config
location = /t {
content_by_lua_block {
local mlcache = require "resty.mlcache"
local ok, err = pcall(mlcache.new, "cache", 200, "")
local ok, err = pcall(mlcache.new, "cache", {
ttl = ""
})
if not ok then
ngx.log(ngx.ERR, err)
end
Expand All @@ -106,26 +132,52 @@ ttl must be a number



=== TEST 5: new() creates an mlcache object with defaults
=== TEST 6: new() validates neg_ttl
--- http_config eval: $::HttpConfig
--- config
location = /t {
content_by_lua_block {
local mlcache = require "resty.mlcache"
local cache, err = mlcache.new("cache", 200)
local ok, err = pcall(mlcache.new, "cache", {
neg_ttl = ""
})
if not ok then
ngx.log(ngx.ERR, err)
end
}
}
--- request
GET /t
--- response_body

--- error_log
neg_ttl must be a number



=== TEST 7: new() creates an mlcache object with defaults
--- http_config eval: $::HttpConfig
--- config
location = /t {
content_by_lua_block {
local mlcache = require "resty.mlcache"
local cache, err = mlcache.new("cache")
if not cache then
ngx.log(ngx.ERR, err)
end
ngx.say(type(cache))
ngx.say(type(cache.ttl))
ngx.say(type(cache.neg_ttl))
}
}
--- request
GET /t
--- response_body
table
number
number
--- no_error_log
[error]
Loading

0 comments on commit efe4b70

Please sign in to comment.