From 982118f22c9f8558f1c65cce833ada08ebbdaa52 Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Wed, 31 Jul 2024 15:55:18 +0800 Subject: [PATCH 1/4] fix: the timer should not be running early. This PR fixes the error of timer running early. When calculating steps, discard the part of `self.real_time` that exceeds the resolution, so that the calculation of steps will not count extra points that have not yet expired. Signed-off-by: tzssangglass --- lib/resty/timerng/job.lua | 5 +- lib/resty/timerng/wheel/group.lua | 3 + spec/09-jump_the_gun_spec.lua | 98 +++++++++++++++++++++++++++++++ 3 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 spec/09-jump_the_gun_spec.lua diff --git a/lib/resty/timerng/job.lua b/lib/resty/timerng/job.lua index 96802772f..0e5941e65 100644 --- a/lib/resty/timerng/job.lua +++ b/lib/resty/timerng/job.lua @@ -245,9 +245,12 @@ function _M.new(wheels, name, callback, delay, once, debug, argc, argv) job_create_meta(self) end + local create_time = ngx_now() + self.create_time = create_time + if self.name == nil then self.name = string_format("unix_timestamp=%f;counter=%d:meta=%s", - math_floor(ngx_now() * 1000), + math_floor(create_time * 1000), NAME_COUNTER, self.meta.name) diff --git a/lib/resty/timerng/wheel/group.lua b/lib/resty/timerng/wheel/group.lua index 5b29bc153..01d9b4ed0 100644 --- a/lib/resty/timerng/wheel/group.lua +++ b/lib/resty/timerng/wheel/group.lua @@ -1,3 +1,5 @@ +local math_floor = math.floor + local utils = require("resty.timerng.utils") local wheel = require("resty.timerng.wheel") local array = require("resty.timerng.array") @@ -103,6 +105,7 @@ function _M:sync_time() return end + self.real_time = math_floor(self.real_time / resolution) * resolution local delta = self.real_time - self.expected_time local steps = utils_convert_second_to_step(delta, resolution) diff --git a/spec/09-jump_the_gun_spec.lua b/spec/09-jump_the_gun_spec.lua new file mode 100644 index 000000000..3247eff8a --- /dev/null +++ b/spec/09-jump_the_gun_spec.lua @@ -0,0 +1,98 @@ + +local timer_module = require("resty.timerng") +local helper = require("helper") + +local sleep = ngx.sleep +local update_time = ngx.update_time +local now = ngx.now +local timer_running_count = ngx.timer.running_count +local string_format = string.format + + +local function callback_func(premature, create_time, delay) + if premature then + return + end + + update_time() + local now = now() + local dict = ngx.shared["timer_jump_the_gun"] + if not dict then + error("not found shared dict: timer_jump_the_gun") + return + end + + dict:set("not_jump_the_gun", now - delay > create_time) +end + + +local function every_func() + update_time() +end + +insulate("timer jump the gun", function () + local timer = { } + + randomize() + + lazy_setup(function () + timer = timer_module.new({ + min_threads = 16, + max_threads = 32, + }) + + assert(timer:start()) + + end) + + lazy_teardown(function () + timer:freeze() + timer:destroy() + + helper.wait_until(function () + assert.same(1, timer_running_count()) + return true + end) + + end) + + after_each(function () + assert.has_no.errors(function () + timer:cancel(helper.TIMER_NAME_ONCE) + end) + end) + + it("test", function () + local delay = 50 * helper.RESOLUTION + local interval = 10 * helper.RESOLUTION + update_time() + + assert.has_no.errors(function () + assert( + timer:every(interval, every_func) + ) + end) + + ngx.sleep(0.999) + + update_time() + assert.has_no.errors(function () + local create_time = now() + assert( + timer:named_at("foo", delay, callback_func, create_time, delay) + ) + end) + + sleep(6) + + local dict = ngx.shared["timer_jump_the_gun"] + if not dict then + error("not found shared dict: timer_jump_the_gun") + return + end + + local jump_the_gun = dict:get("not_jump_the_gun") + assert.is_not_nil(jump_the_gun) + assert.is_true(jump_the_gun) + end) +end) From f1f19cc1196e6601f25b505a94f71f91bbf61e61 Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Tue, 6 Aug 2024 15:16:55 +0800 Subject: [PATCH 2/4] add one more step for once timer Signed-off-by: tzssangglass --- .github/workflows/test.yml | 7 +- lib/resty/timerng/job.lua | 4 + lib/resty/timerng/wheel/group.lua | 1 - spec/09-jump_the_gun_spec.lua | 137 +++++++++++++++++------------- 4 files changed, 86 insertions(+), 63 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f8a3312c0..4c4b13275 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -57,6 +57,7 @@ jobs: - "spec/01*.lua spec/04*.lua spec/05*.lua spec/06*.lua spec/07*.lua spec/08*.lua" - "spec/02*.lua" - "spec/03*.lua" + - "spec/09*.lua" openresty_version: - "1.19.9.1" @@ -116,7 +117,11 @@ jobs: - name: Tests run: | eval $(luarocks path) - resty -I lib -I spec spec/runner.lua --coverage --verbose -o htest --shuffle-tests ${{ matrix.busted_args }} + if ${{ matrix.busted_args }} == "spec/09*.lua" + resty --shdict 'timer_jump_the_gun 1m' -I lib -I spec spec/runner.lua --coverage --verbose -o htest --shuffle-tests ${{ matrix.busted_args }} + else + resty -I lib -I spec spec/runner.lua --coverage --verbose -o htest --shuffle-tests ${{ matrix.busted_args }} + fi - name: Show coverage run: | diff --git a/lib/resty/timerng/job.lua b/lib/resty/timerng/job.lua index 0e5941e65..99e3c6559 100644 --- a/lib/resty/timerng/job.lua +++ b/lib/resty/timerng/job.lua @@ -241,6 +241,10 @@ function _M.new(wheels, name, callback, delay, once, debug, argc, argv) }, } + if once then + self.steps = self.steps + 1 + end + if debug then job_create_meta(self) end diff --git a/lib/resty/timerng/wheel/group.lua b/lib/resty/timerng/wheel/group.lua index 01d9b4ed0..06b55de18 100644 --- a/lib/resty/timerng/wheel/group.lua +++ b/lib/resty/timerng/wheel/group.lua @@ -105,7 +105,6 @@ function _M:sync_time() return end - self.real_time = math_floor(self.real_time / resolution) * resolution local delta = self.real_time - self.expected_time local steps = utils_convert_second_to_step(delta, resolution) diff --git a/spec/09-jump_the_gun_spec.lua b/spec/09-jump_the_gun_spec.lua index 3247eff8a..545a2f3c0 100644 --- a/spec/09-jump_the_gun_spec.lua +++ b/spec/09-jump_the_gun_spec.lua @@ -30,69 +30,84 @@ local function every_func() update_time() end -insulate("timer jump the gun", function () - local timer = { } - - randomize() - - lazy_setup(function () - timer = timer_module.new({ - min_threads = 16, - max_threads = 32, - }) - - assert(timer:start()) - - end) - - lazy_teardown(function () - timer:freeze() - timer:destroy() - - helper.wait_until(function () - assert.same(1, timer_running_count()) - return true +local rand_intervals = { + 0.111, + 0.222, + 0.333, + 0.444, + 0.555, + 0.666, + 0.777, + 0.888, + 0.999, +} + +for rand_interval in pairs(rand_intervals) do + insulate("timer jump the gun", function () + local timer = { } + + randomize() + + lazy_setup(function () + timer = timer_module.new({ + min_threads = 16, + max_threads = 32, + }) + + assert(timer:start()) + end) - - end) - - after_each(function () - assert.has_no.errors(function () - timer:cancel(helper.TIMER_NAME_ONCE) + + lazy_teardown(function () + timer:freeze() + timer:destroy() + + helper.wait_until(function () + assert.same(1, timer_running_count()) + return true + end) + end) - end) - - it("test", function () - local delay = 50 * helper.RESOLUTION - local interval = 10 * helper.RESOLUTION - update_time() - - assert.has_no.errors(function () - assert( - timer:every(interval, every_func) - ) + + after_each(function () + assert.has_no.errors(function () + timer:cancel(helper.TIMER_NAME_ONCE) + end) end) - - ngx.sleep(0.999) - - update_time() - assert.has_no.errors(function () - local create_time = now() - assert( - timer:named_at("foo", delay, callback_func, create_time, delay) - ) + + it("test", function () + local delay = 30 * helper.RESOLUTION + local interval = 10 * helper.RESOLUTION + update_time() + + assert.has_no.errors(function () + assert( + timer:every(interval, every_func) + ) + end) + + sleep(rand_interval) + + update_time() + assert.has_no.errors(function () + local create_time = now() + assert( + timer:named_at("foo", delay, callback_func, create_time, delay) + ) + end) + + sleep(4) + + local dict = ngx.shared["timer_jump_the_gun"] + if not dict then + error("not found shared dict: timer_jump_the_gun") + return + end + + local jump_the_gun = dict:get("not_jump_the_gun") + assert.is_not_nil(jump_the_gun) + assert.is_true(jump_the_gun) end) - - sleep(6) - - local dict = ngx.shared["timer_jump_the_gun"] - if not dict then - error("not found shared dict: timer_jump_the_gun") - return - end - - local jump_the_gun = dict:get("not_jump_the_gun") - assert.is_not_nil(jump_the_gun) - assert.is_true(jump_the_gun) end) -end) +end + From 197a6596d9276db0d3e729c366be5cee6bb4fbbc Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Tue, 6 Aug 2024 15:19:55 +0800 Subject: [PATCH 3/4] chore Signed-off-by: tzssangglass --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4c4b13275..55f3d14ba 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -117,7 +117,7 @@ jobs: - name: Tests run: | eval $(luarocks path) - if ${{ matrix.busted_args }} == "spec/09*.lua" + if ${{ matrix.busted_args }} == "spec/09*.lua" then resty --shdict 'timer_jump_the_gun 1m' -I lib -I spec spec/runner.lua --coverage --verbose -o htest --shuffle-tests ${{ matrix.busted_args }} else resty -I lib -I spec spec/runner.lua --coverage --verbose -o htest --shuffle-tests ${{ matrix.busted_args }} From 49f14a8e6f5515109b291ca351e598528a7af112 Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Tue, 6 Aug 2024 15:23:50 +0800 Subject: [PATCH 4/4] fix Signed-off-by: tzssangglass --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 55f3d14ba..bf9cd16f3 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -117,7 +117,7 @@ jobs: - name: Tests run: | eval $(luarocks path) - if ${{ matrix.busted_args }} == "spec/09*.lua" then + if [[ "${{ matrix.busted_args }}" == "spec/09*.lua" ]]; then resty --shdict 'timer_jump_the_gun 1m' -I lib -I spec spec/runner.lua --coverage --verbose -o htest --shuffle-tests ${{ matrix.busted_args }} else resty -I lib -I spec spec/runner.lua --coverage --verbose -o htest --shuffle-tests ${{ matrix.busted_args }}