Skip to content

keep the key never overrides #6

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

keep the key never overrides #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e178edd
keep the key never overrides
Dec 4, 2015
61760d6
add an option for safe_add
membphis Dec 6, 2015
19d48da
fix the typos
membphis Dec 6, 2015
f988311
add test case
membphis Dec 6, 2015
2665d2a
modify the format
Dec 7, 2015
3a39083
add another two test cases
Dec 7, 2015
96640db
change the test case title
Dec 7, 2015
3589314
declare the variable at first
Dec 14, 2015
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
9 changes: 6 additions & 3 deletions lib/resty/lock.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,14 @@ function _M.new(_, dict_name, opts)
cdata.key_id = 0
cdata.dict_id = ref_obj(dict)

local timeout, exptime, step, ratio, max_step
local timeout, exptime, step, ratio, max_step, safe_add
if opts then
timeout = opts.timeout
exptime = opts.exptime
step = opts.step
ratio = opts.ratio
max_step = opts.max_step
safe_add = opts.safe_add
Copy link

@moonbingbing moonbingbing Dec 14, 2015

Choose a reason for hiding this comment

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

safe_add is NOT local var

end

if not exptime then
Expand All @@ -107,6 +108,7 @@ function _M.new(_, dict_name, opts)
local self = {
cdata = cdata,
dict = dict,
dict_add = safe_add and dict.safe_add or dict.add,
Copy link

@moonbingbing moonbingbing Dec 14, 2015

Choose a reason for hiding this comment

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

Code readability is not good enough.
how about this:
local dict_add = dict.add
if safe_add then
dict_add = dict.safe_add
end

timeout = timeout or 5,
exptime = exptime,
step = step or 0.001,
Expand All @@ -128,7 +130,8 @@ function _M.lock(self, key)
return nil, "locked"
end
local exptime = self.exptime
local ok, err = dict:add(key, true, exptime)
local dict_add = self.dict_add
local ok, err = dict_add(dict, key, true, exptime)
if ok then
cdata.key_id = ref_obj(key)
if not shdict_mt then
Expand All @@ -154,7 +157,7 @@ function _M.lock(self, key)
elapsed = elapsed + step
timeout = timeout - step

local ok, err = dict:add(key, true, exptime)
local ok, err = dict_add(dict, key, true, exptime)
if ok then
cdata.key_id = ref_obj(key)
if not shdict_mt then
Expand Down
133 changes: 133 additions & 0 deletions t/sanity.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,3 +468,136 @@ lock 2: unlock: nil, unlocked
--- no_error_log
[error]



=== TEST 14: serial lock and unlock with safe_add
--- http_config eval: $::HttpConfig
--- config
location = /t {
content_by_lua '
local lock = require "resty.lock"
for i = 1, 2 do
local lock = lock:new("cache_locks", {safe_add = true})
local elapsed, err = lock:lock("foo")
ngx.say("lock: ", elapsed, ", ", err)
local ok, err = lock:unlock()
if not ok then
ngx.say("failed to unlock: ", err)
end
ngx.say("unlock: ", ok)
end
';
}
--- request
GET /t
--- response_body
lock: 0, nil
unlock: 1
lock: 0, nil
unlock: 1
Copy link
Member

@agentzh agentzh Dec 6, 2015

Choose a reason for hiding this comment

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

Need to test the case that safe_add actually returned the "no memory" error.

Copy link
Author

@membphis membphis Dec 6, 2015

Choose a reason for hiding this comment

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

added the new test case . ^_^

On Sun, Dec 6, 2015 at 12:45 PM, Yichun Zhang [email protected]
wrote:

In t/sanity.t
#6 (comment):

  •            ngx.say("lock: ", elapsed, ", ", err)

  •            local ok, err = lock:unlock()

  •            if not ok then

  •                ngx.say("failed to unlock: ", err)

  •            end

  •            ngx.say("unlock: ", ok)

  •        end

  •    ';
  • }
    +--- request
    +GET /t
    +--- response_body
    +lock: 0, nil
    +unlock: 1
    +lock: 0, nil
    +unlock: 1

Need to test the case that safe_add actually returned the "no memory"
error.

—
Reply to this email directly or view it on GitHub
https://github.com/openresty/lua-resty-lock/pull/6/files#r46766560.

MembPhis
My github: https://github.com/membphis
Our Book: OpenResty Best Practices
https://www.gitbook.com/book/moonbingbing/openresty-best-practices


--- no_error_log
[error]



=== TEST 15: the "safe_add" option is true: exhausting the shm zone memory
--- http_config eval: $::HttpConfig
--- config
location = /t {
content_by_lua '
local lock = require "resty.lock"
local cache = ngx.shared.cache_locks

cache:flush_all()
function full_dict( dict )
for i = 1, 10000 do
local ok, err = dict:safe_set(string.rep("2", 2) .. i, string.rep("2", 5) .. i)
if not ok then
return
end
end
end
full_dict(cache)

local lock = lock:new("cache_locks", {safe_add = true, timeout = 0})
local elapsed, err = lock:lock("foo")
ngx.say("lock: ", elapsed, ", ", err)
';
}
--- request
GET /t
--- response_body
lock: nil, no memory

--- no_error_log
[error]



=== TEST 16: the "safe_add" option is false: exhausting the shm zone memory
--- http_config eval: $::HttpConfig
--- config
location = /t {
content_by_lua '
local lock = require "resty.lock"
local cache = ngx.shared.cache_locks

cache:flush_all()
function full_dict( dict )
for i = 1, 10000 do
local ok, err = dict:safe_set(string.rep("2", 2) .. i, string.rep("2", 5) .. i)
if not ok then
return
end
end
end
full_dict(cache)

local lock = lock:new("cache_locks", {safe_add = false, timeout = 0})
local elapsed, err = lock:lock("foo")
ngx.say("lock: ", elapsed, ", ", err)
';
}
--- request
GET /t
--- response_body
lock: 0, nil

--- no_error_log
[error]



Copy link
Member

@agentzh agentzh Dec 6, 2015

Choose a reason for hiding this comment

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

Style: let's remove these file trailing new lines :)

Copy link
Author

@membphis membphis Dec 7, 2015

Choose a reason for hiding this comment

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

There is 3 blank lines between different test cases. Do i need to remove the line #503 ?

Copy link
Member

@agentzh agentzh Dec 7, 2015

Choose a reason for hiding this comment

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

@membphis These 3 blank lines were at the end of the file. And only in that case, we should remove them :)

Copy link
Author

@membphis membphis Dec 7, 2015

Choose a reason for hiding this comment

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

fine, i have fixed this before.

=== TEST 17: the "safe_add" option is off by default: exhausting the shm zone memory
--- http_config eval: $::HttpConfig
--- config
location = /t {
content_by_lua '
local lock = require "resty.lock"
local cache = ngx.shared.cache_locks

cache:flush_all()
function full_dict( dict )
for i = 1, 10000 do
local ok, err = dict:safe_set(string.rep("2", 2) .. i, string.rep("2", 5) .. i)
if not ok then
return
end
end
end
full_dict(cache)

local lock = lock:new("cache_locks", {timeout = 0})
local elapsed, err = lock:lock("foo")
ngx.say("lock: ", elapsed, ", ", err)
';
}
--- request
GET /t
--- response_body
lock: 0, nil

--- no_error_log
[error]