Skip to content

Commit

Permalink
fix(bk-cache-fallback): enhance error messages for lock acquisition f…
Browse files Browse the repository at this point in the history
…ailures and add tests for timeout scenarios
  • Loading branch information
wklken committed Nov 12, 2024
1 parent bdf9271 commit f3d9b23
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 5 deletions.
7 changes: 4 additions & 3 deletions src/apisix/plugins/bk-cache-fallback/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ function _M.get_with_fallback(self, ctx, key, version, create_obj_func, ...)
local elapsed, lock_err = lock:lock(key_s)
if not elapsed then
if lock_err ~= "timeout" then
return nil, "failed to acquire the bk-cache-fallback lock, err: " .. lock_err
return nil, "failed to acquire the bk-cache-fallback lock, key: " .. key_s .. ", err: " .. lock_err
end

-- NOTE: 2024-11-11 we met some timeout here, in the same apisix pod, the same cache_key,
Expand All @@ -151,13 +151,14 @@ function _M.get_with_fallback(self, ctx, key, version, create_obj_func, ...)
if sd ~= nil then
local obj_decoded, json_err = core.json.decode(sd)
if json_err == nil then
log.error("failed to acquire the bk-cache-fallback lock, fallback to get the data from shared_dict")
log.error("failed to acquire the bk-cache-fallback lock, key: " .. key_s ..
", fallback to get the data from shared_dict")
return obj_decoded, nil
end
end
end

return nil, "failed to acquire the bk-cache-fallback lock, error: timeout."
return nil, "failed to acquire the bk-cache-fallback lock, key: " .. key_s .. ", error: timeout."
end

-- TODO: 函数过长, 需要考虑拆分, 特别是 unlock 特别多, 也容易出问题
Expand Down
120 changes: 120 additions & 0 deletions src/apisix/tests/bk-cache-fallback/test-init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
local cache_fallback = require("apisix.plugins.bk-cache-fallback.init")
local ngx_shared = ngx.shared
local ngx_sleep = ngx.sleep
local core = require("apisix.core")

describe(
"bk-cache-fallback", function()
Expand Down Expand Up @@ -313,5 +314,124 @@ describe(
end
)

-- a new case here, to test the lock:lock timeout and other errors
describe("lock:lock timeout and other errors", function()
local key
local f

before_each(function ()
key = "foo"

f = {
create_obj_func_ok = function() end,
create_obj_func_fail = function() end,
}

stub(f, "create_obj_func_ok", function ()
return {
["hello"] = "world",
}, nil
end)
stub(f, "create_obj_func_fail", function ()
return nil, "error"
end)

end)

after_each(function ()
f.create_obj_func_ok:revert()
f.create_obj_func_fail:revert()
end)

it("is not timeout error, should return the error", function ()
local ctx = {
conf_id = 789,
conf_type = "test",
}
local cache = cache_fallback.new(conf, "bk-cache-fallback-lock-error")
local cache_key = cache.key_prefix .. key

local shared_data_dict = ngx_shared[cache.plugin_shared_dict_name]
assert.is_not_nil(shared_data_dict)

-- Stub lock:lock to simulate an error (not timeout)
local lock = require("resty.lock")
stub(lock, "lock", function(self, key_s)
if key_s == cache_key then
return nil, "some error"
else
return 0, nil
end
end)

local obj, err = cache:get_with_fallback(ctx, key, nil, f.create_obj_func_fail)
assert.is_nil(obj)
assert.is_not_nil(err)
assert.equal("failed to acquire the bk-cache-fallback lock, key: bk-cache-fallback-lock-error:foo, err: some error", err)

lock.lock:revert()
end)

it("is timeout, no data in shared_dict, return the error", function ()
local ctx = {
conf_id = 789,
conf_type = "test",
}
local cache = cache_fallback.new(conf, "bk-cache-fallback-lock-timeout")
local cache_key = cache.key_prefix .. key

local shared_data_dict = ngx_shared[cache.plugin_shared_dict_name]
assert.is_not_nil(shared_data_dict)

-- Stub lock:lock to simulate a timeout
local lock = require("resty.lock")
stub(lock, "lock", function(self, key_s)
if key_s == cache_key then
return nil, "timeout"
else
return 0, nil
end
end)

local obj, err = cache:get_with_fallback(ctx, key, nil, f.create_obj_func_fail)
assert.is_nil(obj)
assert.is_not_nil(err)
assert.equal("failed to acquire the bk-cache-fallback lock, key: bk-cache-fallback-lock-timeout:foo, error: timeout.", err)

lock.lock:revert()
end)

it("is timeout, got the data in shared_dict, return data,nil", function ()
local ctx = {
conf_id = 789,
conf_type = "test",
}
local cache = cache_fallback.new(conf, "bk-cache-fallback-lock-timeout-data")
local cache_key = cache.key_prefix .. key

local shared_data_dict = ngx_shared[cache.plugin_shared_dict_name]
assert.is_not_nil(shared_data_dict)

-- set data in shared_dict
shared_data_dict:set(cache_key, '{"hello":"abc"}', 60 * 60)

-- Stub lock:lock to simulate a timeout
local lock = require("resty.lock")
stub(lock, "lock", function(self, key_s)
if key_s == cache_key then
return nil, "timeout"
else
return 0, nil
end
end)

local obj, err = cache:get_with_fallback(ctx, key, nil, f.create_obj_func_ok)
assert.is_not_nil(obj)
assert.is_nil(err)
assert.equal("abc", obj["hello"])

lock.lock:revert()
end)
end)
end
)
7 changes: 5 additions & 2 deletions src/apisix/tests/conf/nginx.conf
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@ lua_shared_dict prometheus-metrics 1m;
lua_shared_dict plugin-bk-permission 1m;
lua_shared_dict plugin-bk-cache-fallback 1m;
lua_shared_dict plugin-bk-cache-fallback-lock 1m;
# for unittest, bk-cache-fallback/init.lua, case ok/fail/ok->fail
# for unittest, bk-cache-fallback/init.lua, case ok/fail/ok->fail/lock fail
lua_shared_dict plugin-bk-cache-fallback-ok 1m;
lua_shared_dict plugin-bk-cache-fallback-fail 1m;
lua_shared_dict plugin-bk-cache-fallback-ok-fail 1m;
lua_shared_dict plugin-bk-cache-fallback-lock-error 1m;
lua_shared_dict plugin-bk-cache-fallback-lock-timeout 1m;
lua_shared_dict plugin-bk-cache-fallback-lock-timeout-data 1m;

error_log /bkgateway/logs/error.log;
error_log /bkgateway/logs/error.log;

0 comments on commit f3d9b23

Please sign in to comment.