fix: fix delay clean logic when multiple healthchecker was started (#147

) * fix: fix delay clean logic when multiple healthchecker was started * fix: add test * fix: remove old os action because github action have been not support * fix: fix code
Kong · Dec 22, 2023 · 4972646 · 4972646
1 parent b3901c2
commit 4972646
Show file tree

Hide file tree

Showing 3 changed files with 89 additions and 192 deletions.
diff --git a/.github/workflows/old_os.yml b/.github/workflows/old_os.yml
diff --git a/lib/resty/healthcheck.lua b/lib/resty/healthcheck.lua
@@ -1652,11 +1652,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
@@ -1683,8 +1685,6 @@ function _M.new(opts)
                 end
               end
             end)
-
-            last_cleanup_check = cur_time
           end
 
           if checker_obj.checks.active.healthy.active and
@@ -1703,6 +1703,9 @@ 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

diff --git a/t/11-clear.t b/t/11-clear.t
@@ -3,7 +3,7 @@ use Cwd qw(cwd);
 
 workers(1);
 
-plan tests => repeat_each() * 27;
+plan tests => repeat_each() * 27 + 2;
 
 my $pwd = cwd();
 
@@ -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