Skip to content

feature: shdict:incr(): added the "exptime" argument to set the expiry of values when they are first created via the "init" argument. #165

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

feature: shdict:incr(): added the "exptime" argument to set the expiry of values when they are first created via the "init" argument. #165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ install:
- git clone https://github.com/openresty/openresty.git ../openresty
- git clone https://github.com/openresty/openresty-devel-utils.git
- git clone https://github.com/simpl/ngx_devel_kit.git ../ndk-nginx-module
- git clone https://github.com/openresty/lua-nginx-module.git ../lua-nginx-module
- git clone -b feat/incr-exptime-ffi https://github.com/thibaultcha/lua-nginx-module.git ../lua-nginx-module
- git clone https://github.com/openresty/no-pool-nginx.git ../no-pool-nginx
- git clone https://github.com/openresty/echo-nginx-module.git ../echo-nginx-module
- git clone https://github.com/openresty/lua-resty-lrucache.git
Expand Down
33 changes: 24 additions & 9 deletions lib/resty/core/shdict.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ ffi.cdef[[

int ngx_http_lua_ffi_shdict_incr(void *zone, const unsigned char *key,
size_t key_len, double *value, char **err, int has_init, double init,
int *forcible);
long init_ttl, int *forcible);

int ngx_http_lua_ffi_shdict_store(void *zone, int op,
const unsigned char *key, size_t key_len, int value_type,
Expand Down Expand Up @@ -337,7 +337,7 @@ local function shdict_get_stale(zone, key)
end


local function shdict_incr(zone, key, value, init)
local function shdict_incr(zone, key, value, init, init_ttl)
zone = check_zone(zone)

if key == nil then
Expand All @@ -361,8 +361,6 @@ local function shdict_incr(zone, key, value, init)
end
num_value[0] = value

local has_init

if init then
local typ = type(init)
if typ ~= "number" then
Expand All @@ -372,22 +370,39 @@ local function shdict_incr(zone, key, value, init)
return error("bad init arg: number expected, got " .. typ)
end
end
end

has_init = 1
if init_ttl ~= nil then
local typ = type(init_ttl)
if typ ~= "number" then
init_ttl = tonumber(init_ttl)

if not init_ttl then
error("bad init_ttl arg: number expected, got " .. typ, 2)
end
end

if init_ttl < 0 then
error('bad "init_ttl" argument', 2)
end

if not init then
error('must provide "init" when providing "init_ttl"', 2)
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really happy with such complex argument value check. It looks branchy and expensive. Will you simplify it in a future PR? Thanks!


else
has_init = 0
init = 0
init_ttl = 0
end

local rc = C.ngx_http_lua_ffi_shdict_incr(zone, key, key_len, num_value,
errmsg, has_init, init,
errmsg, init and 1 or 0,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I now see why it used has_init as an integer in the first place...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes... However, this patch still looks like an improvement to me though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thibaultcha I didn't disagree :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😅

init or 0, init_ttl * 1000,
forcible)
if rc ~= 0 then -- ~= NGX_OK
return nil, ffi_str(errmsg[0])
end

if has_init == 0 then
if not init then
return tonumber(num_value[0])
end

Expand Down
244 changes: 244 additions & 0 deletions t/shdict.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1284,3 +1284,247 @@ free_page_bytes: (?:0|32768)
[error]
[alert]
[crit]



=== TEST 39: incr bad init_ttl argument
--- http_config eval: $::HttpConfig
--- config
location = /t {
content_by_lua_block {
local dogs = ngx.shared.dogs
local pok, err = pcall(dogs.incr, dogs, "foo", 1, 0, -1)
if not pok then
ngx.say("not ok: ", err)
return
end

ngx.say("ok")
}
}
--- request
GET /t
--- response_body
not ok: bad "init_ttl" argument
--- no_error_log
[error]
[alert]
[crit]



=== TEST 40: incr init_ttl argument is not a number
--- http_config eval: $::HttpConfig
--- config
location = /t {
content_by_lua_block {
local dogs = ngx.shared.dogs
local pok, err = pcall(dogs.incr, dogs, "foo", 1, 0, "bar")
if not pok then
ngx.say("not ok: ", err)
return
end

ngx.say("ok")
}
}
--- request
GET /t
--- response_body
not ok: bad init_ttl arg: number expected, got string
--- no_error_log
[error]
[alert]
[crit]



=== TEST 41: incr init_ttl argument without init
--- http_config eval: $::HttpConfig
--- config
location = /t {
content_by_lua_block {
local dogs = ngx.shared.dogs
local pok, err = pcall(dogs.incr, dogs, "foo", 1, nil, 0.001)
if not pok then
ngx.say("not ok: ", err)
return
end

ngx.say("ok")
}
}
--- request
GET /t
--- response_body
not ok: must provide "init" when providing "init_ttl"
--- no_error_log
[error]
[alert]
[crit]



=== TEST 42: incr key with init_ttl (key exists)
--- http_config eval: $::HttpConfig
--- config
location = /t {
content_by_lua_block {
local dogs = ngx.shared.dogs
dogs:set("foo", 32)

local res, err = dogs:incr("foo", 10502, 0, 0.001)
ngx.say("incr: ", res, " ", err)
ngx.say("foo = ", dogs:get("foo"))

ngx.sleep(0.002)

ngx.say("foo after incr init_ttl = ", dogs:get("foo"))
}
}
--- request
GET /t
--- response_body
incr: 10534 nil
foo = 10534
foo after incr init_ttl = 10534
--- no_error_log
[error]
[alert]
[crit]



=== TEST 43: incr key with init and init_ttl (key not exists)
--- http_config eval: $::HttpConfig
--- config
location = /t {
content_by_lua_block {
local dogs = ngx.shared.dogs
dogs:flush_all()

local res, err = dogs:incr("foo", 10502, 1, 0.001)
ngx.say("incr: ", res, " ", err)
ngx.say("foo = ", dogs:get("foo"))

ngx.sleep(0.002)

ngx.say("foo after init_ttl = ", dogs:get("foo"))
}
}
--- request
GET /t
--- response_body
incr: 10503 nil
foo = 10503
foo after init_ttl = nil
--- no_error_log
[error]
[alert]
[crit]



=== TEST 44: incr key with init and init_ttl as string (key not exists)
--- http_config eval: $::HttpConfig
--- config
location = /t {
content_by_lua_block {
local dogs = ngx.shared.dogs
dogs:flush_all()

local res, err = dogs:incr("foo", 10502, 1, "0.001")
ngx.say("incr: ", res, " ", err)
ngx.say("foo = ", dogs:get("foo"))

ngx.sleep(0.002)

ngx.say("foo after init_ttl = ", dogs:get("foo"))
}
}
--- request
GET /t
--- response_body
incr: 10503 nil
foo = 10503
foo after init_ttl = nil
--- no_error_log
[error]
[alert]
[crit]



=== TEST 45: incr key with init and init_ttl (key expired and size matched)
--- http_config eval: $::HttpConfig
--- config
location = /t {
content_by_lua_block {
local dogs = ngx.shared.dogs
for i = 1, 20 do
dogs:set("bar" .. i, i, 0.002)
end
dogs:set("foo", 32, 0.002)
ngx.sleep(0.003)

local res, err = dogs:incr("foo", 10502, 0, 0.001)
ngx.say("incr: ", res, " ", err)
ngx.say("foo = ", dogs:get("foo"))

ngx.sleep(0.002)

ngx.say("foo after init_ttl = ", dogs:get("foo"))
}
}
--- request
GET /t
--- response_body
incr: 10502 nil
foo = 10502
foo after init_ttl = nil
--- no_error_log
[error]
[alert]
[crit]



=== TEST 46: incr key with init and init_ttl (forcibly override other valid entries)
--- http_config eval: $::HttpConfig
--- config
location = /t {
content_by_lua_block {
local dogs = ngx.shared.dogs
dogs:flush_all()

local long_prefix = string.rep("1234567890", 100)
for i = 1, 1000 do
local success, err, forcible = dogs:set(long_prefix .. i, i)
if forcible then
dogs:delete(long_prefix .. i)
break
end
end

local res, err, forcible = dogs:incr(long_prefix .. "bar", 10502, 0)
ngx.say("incr: ", res, " ", err, " ", forcible)

local res, err, forcible = dogs:incr(long_prefix .. "foo", 10502, 0, 0.001)
ngx.say("incr: ", res, " ", err, " ", forcible)
ngx.say("foo = ", dogs:get(long_prefix .. "foo"))

ngx.sleep(0.002)
ngx.say("foo after init_ttl = ", dogs:get("foo"))
}
}
--- request
GET /t
--- response_body
incr: 10502 nil false
incr: 10502 nil true
foo = 10502
foo after init_ttl = nil
--- no_error_log
[error]
[alert]
[crit]