Skip to content

Commit

Permalink
fix: fix delay clean logic when multiple healthchecker was started (#146
Browse files Browse the repository at this point in the history
)

* fix: fix delay clean logic when multiple healthchecker was started

* fix: add test

* fix: add test

* fix: add test

* fix: fix code

* fix: fix code

* fix: fix code
  • Loading branch information
oowl authored Dec 22, 2023
1 parent 7478c5f commit 94c3bba
Show file tree
Hide file tree
Showing 5 changed files with 178 additions and 7 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build_and_test_with_resty_events.yml
Original file line number Diff line number Diff line change
Expand Up @@ -147,4 +147,4 @@ jobs:
run: |
eval `luarocks path`
eval $(perl -I $HOME/perl5/lib/perl5/ -Mlocal::lib)
TEST_NGINX_RANDOMIZE=1 prove -I. -r t/with_resty-events
TEST_NGINX_TIMEOUT=4 TEST_NGINX_RANDOMIZE=1 prove -I. -r t/with_resty-events
4 changes: 2 additions & 2 deletions .github/workflows/build_and_test_with_worker_events.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
runs-on: ubuntu-20.04
strategy:
matrix:
openresty-version: [1.19.9.1, 1.21.4.1]
openresty-version: [1.21.4.1]

steps:
- name: Update and install OS dependencies
Expand Down Expand Up @@ -72,4 +72,4 @@ jobs:
run: |
eval `luarocks path`
eval $(perl -I $HOME/perl5/lib/perl5/ -Mlocal::lib)
TEST_NGINX_RANDOMIZE=1 prove -I. -r t/with_worker-events
TEST_NGINX_TIMEOUT=4 TEST_NGINX_RANDOMIZE=1 prove -I. -r t/with_worker-events
8 changes: 6 additions & 2 deletions lib/resty/healthcheck.lua
Original file line number Diff line number Diff line change
Expand Up @@ -1690,11 +1690,13 @@ function _M.new(opts)
end

local cur_time = ngx_now()
local is_checked = false
for _, checker_obj in pairs(hcs) do

if (last_cleanup_check + CLEANUP_INTERVAL) < cur_time then
-- clear targets marked for delayed removal
locking_target_list(checker_obj, function(target_list)
is_checked = true
local removed_targets = {}
local index = 1
while index <= #target_list do
Expand All @@ -1721,8 +1723,6 @@ function _M.new(opts)
end
end
end)

last_cleanup_check = cur_time
end

if checker_obj.checks.active.healthy.active and
Expand All @@ -1741,6 +1741,10 @@ function _M.new(opts)
checker_callback(checker_obj, "unhealthy")
end
end

if is_checked then
last_cleanup_check = cur_time
end
end,
})
if not active_check_timer then
Expand Down
86 changes: 85 additions & 1 deletion t/with_resty-events/11-clear.t
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use Cwd qw(cwd);

workers(1);

plan tests => repeat_each() * 27;
plan tests => repeat_each() * 27 + 2;

my $pwd = cwd();
$ENV{TEST_NGINX_SERVROOT} = server_root();
Expand Down Expand Up @@ -296,3 +296,87 @@ false
target not found
false
target not found

=== TEST 6: delayed_clear() would clear tgt list when we add two checkers
--- http_config eval: $::HttpConfig
--- config
location = /t {
content_by_lua_block {
local healthcheck = require("resty.healthcheck")
local config1 = {
name = "testing",
shm_name = "test_shm",
events_module = "resty.events",
checks = {
active = {
healthy = {
interval = 0.1
},
unhealthy = {
interval = 0.1
}
}
}
}

local config2 = {
name = "testing2",
shm_name = "test_shm",
events_module = "resty.events",
checks = {
active = {
healthy = {
interval = 0.1
},
unhealthy = {
interval = 0.1
}
}
}
}

local checker1 = healthcheck.new(config1)
checker1:add_target("127.0.0.1", 10001, nil, true)
checker1:add_target("127.0.0.1", 10002, nil, true)
checker1:add_target("127.0.0.1", 10003, nil, true)
ngx.sleep(0.2)
ngx.say(checker1:get_target_status("127.0.0.1", 10002))
checker1:delayed_clear(0.2)

local checker2 = healthcheck.new(config2)
checker2:add_target("127.0.0.1", 10001, nil, true)
checker2:add_target("127.0.0.1", 10002, nil, true)
ngx.sleep(0.2)
ngx.say(checker2:get_target_status("127.0.0.1", 10002))
checker2:delayed_clear(0.2)

ngx.sleep(3) -- wait twice the interval

local status, err = checker1:get_target_status("127.0.0.1", 10001)
if status ~= nil then
ngx.say(status)
else
ngx.say(err)
end
status, err = checker2:get_target_status("127.0.0.1", 10002)
if status ~= nil then
ngx.say(status)
else
ngx.say(err)
end
status, err = checker2:get_target_status("127.0.0.1", 10003)
if status ~= nil then
ngx.say(status)
else
ngx.say(err)
end
}
}
--- request
GET /t
--- response_body
true
true
target not found
target not found
target not found
85 changes: 84 additions & 1 deletion t/with_worker-events/11-clear.t
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use Cwd qw(cwd);

workers(1);

plan tests => repeat_each() * 27;
plan tests => repeat_each() * 27 + 2;

my $pwd = cwd();

Expand Down Expand Up @@ -280,3 +280,86 @@ false
target not found
false
target not found


=== TEST 6: delayed_clear() would clear tgt list when we add two checkers
--- http_config eval: $::HttpConfig
--- config
location = /t {
content_by_lua_block {
local we = require "resty.worker.events"
assert(we.configure{ shm = "my_worker_events", interval = 0.1 })
local healthcheck = require("resty.healthcheck")
local config1 = {
name = "testing",
shm_name = "test_shm",
checks = {
active = {
healthy = {
interval = 0.1
},
unhealthy = {
interval = 0.1
}
}
}
}

local config2 = {
name = "testing2",
shm_name = "test_shm",
checks = {
active = {
healthy = {
interval = 0.1
},
unhealthy = {
interval = 0.1
}
}
}
}

local checker1 = healthcheck.new(config1)
checker1:add_target("127.0.0.1", 10001, nil, true)
checker1:add_target("127.0.0.1", 10002, nil, true)
checker1:add_target("127.0.0.1", 10003, nil, true)
ngx.say(checker1:get_target_status("127.0.0.1", 10002))
checker1:delayed_clear(0.2)

local checker2 = healthcheck.new(config2)
checker2:add_target("127.0.0.1", 10001, nil, true)
checker2:add_target("127.0.0.1", 10002, nil, true)
ngx.say(checker2:get_target_status("127.0.0.1", 10002))
checker2:delayed_clear(0.2)

ngx.sleep(3) -- wait twice the interval

local status, err = checker1:get_target_status("127.0.0.1", 10001)
if status ~= nil then
ngx.say(status)
else
ngx.say(err)
end
status, err = checker2:get_target_status("127.0.0.1", 10002)
if status ~= nil then
ngx.say(status)
else
ngx.say(err)
end
status, err = checker2:get_target_status("127.0.0.1", 10003)
if status ~= nil then
ngx.say(status)
else
ngx.say(err)
end
}
}
--- request
GET /t
--- response_body
true
true
target not found
target not found
target not found

0 comments on commit 94c3bba

Please sign in to comment.