From 117fe92263cf7855deaa51ac91c6d3d768c68a7c Mon Sep 17 00:00:00 2001 From: "alexander.klyuchev" Date: Sun, 6 Apr 2025 09:27:43 +0300 Subject: [PATCH 1/9] added flag --- core/include/userver/utils/periodic_task.hpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/include/userver/utils/periodic_task.hpp b/core/include/userver/utils/periodic_task.hpp index 22ba3147ae93..cf0991febc5e 100644 --- a/core/include/userver/utils/periodic_task.hpp +++ b/core/include/userver/utils/periodic_task.hpp @@ -124,6 +124,9 @@ class PeriodicTask final { /// PeriodicTask::Start() calls engine::current_task::GetTaskProcessor() /// to get the TaskProcessor. engine::TaskProcessor* task_processor{nullptr}; + + /// @brief enabled allows to enable/disable timer during run + bool enabled{true}; }; /// Signature of the task to be executed each period. From a4d60b212acb62e3dca3b3707ad199677e1c34b2 Mon Sep 17 00:00:00 2001 From: "alexander.klyuchev" Date: Sun, 6 Apr 2025 10:02:37 +0300 Subject: [PATCH 2/9] added logic --- core/src/utils/periodic_task.cpp | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/core/src/utils/periodic_task.cpp b/core/src/utils/periodic_task.cpp index da947d48e3f8..cd9ade98dc31 100644 --- a/core/src/utils/periodic_task.cpp +++ b/core/src/utils/periodic_task.cpp @@ -98,6 +98,7 @@ void PeriodicTask::Stop() noexcept { } void PeriodicTask::SetSettings(Settings settings) { + bool should_notify_task{}; { auto writer = settings_.StartWrite(); @@ -106,7 +107,7 @@ void PeriodicTask::SetSettings(Settings settings) { return; } settings.flags = writer->flags; - should_notify_task = settings.period != writer->period || settings.exception_period != writer->exception_period; + should_notify_task = settings.period != writer->period || settings.exception_period != writer->exception_period || || settings.enabled != writer->enabled; *writer = std::move(settings); writer.Commit(); } @@ -118,7 +119,12 @@ void PeriodicTask::SetSettings(Settings settings) { } void PeriodicTask::ForceStepAsync() { + should_force_step_ = true; + if (!writer->enabled) { + writer->enabled = true; + writer.Commit(); + } changed_event_.Send(); } @@ -133,6 +139,7 @@ bool PeriodicTask::SynchronizeDebug(bool preserve_span) { bool PeriodicTask::IsRunning() const { return task_.IsValid(); } void PeriodicTask::Run() { + bool skip_step = false; { auto settings = settings_.Read(); @@ -145,11 +152,12 @@ void PeriodicTask::Run() { const auto before = std::chrono::steady_clock::now(); bool no_exception = true; - if (!std::exchange(skip_step, false)) { + const auto settings = settings_.Read(); + bool taskEnabled = settings->enabled; + if (!std::exchange(skip_step, false) && taskEnabled) { no_exception = Step(); } - const auto settings = settings_.Read(); auto period = settings->period; const auto exception_period = settings->exception_period.value_or(period); @@ -162,12 +170,17 @@ void PeriodicTask::Run() { start = std::chrono::steady_clock::now(); } - while (changed_event_.WaitForEventUntil(start + MutatePeriod(period))) { + while ((!taskEnabled && changed_event_.WaitForEvent()) || (taskEnabled && changed_event_.WaitForEventUntil(start + MutatePeriod(period)))) { if (should_force_step_.exchange(false)) { - break; + break; } // The config variable value has been changed, reload const auto settings = settings_.Read(); + taskEnabled = settings->enabled; + if(!taskEnabled) { + break; + } + period = settings->period; const auto exception_period = settings->exception_period.value_or(period); if (!no_exception) period = exception_period; From f28a5177c931872979717e7bddaae6de4594c6af Mon Sep 17 00:00:00 2001 From: "alexander.klyuchev" Date: Sun, 6 Apr 2025 10:31:38 +0300 Subject: [PATCH 3/9] added tests --- core/src/utils/periodic_task.cpp | 3 +-- core/src/utils/periodic_task_test.cpp | 35 +++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/core/src/utils/periodic_task.cpp b/core/src/utils/periodic_task.cpp index cd9ade98dc31..a5f16e26939f 100644 --- a/core/src/utils/periodic_task.cpp +++ b/core/src/utils/periodic_task.cpp @@ -107,7 +107,7 @@ void PeriodicTask::SetSettings(Settings settings) { return; } settings.flags = writer->flags; - should_notify_task = settings.period != writer->period || settings.exception_period != writer->exception_period || || settings.enabled != writer->enabled; + should_notify_task = settings.period != writer->period || settings.exception_period != writer->exception_period || settings.enabled != writer->enabled; *writer = std::move(settings); writer.Commit(); } @@ -139,7 +139,6 @@ bool PeriodicTask::SynchronizeDebug(bool preserve_span) { bool PeriodicTask::IsRunning() const { return task_.IsValid(); } void PeriodicTask::Run() { - bool skip_step = false; { auto settings = settings_.Read(); diff --git a/core/src/utils/periodic_task_test.cpp b/core/src/utils/periodic_task_test.cpp index fba863a8fd20..a070290358c8 100644 --- a/core/src/utils/periodic_task_test.cpp +++ b/core/src/utils/periodic_task_test.cpp @@ -418,4 +418,39 @@ UTEST_F(PeriodicTaskLog, ErrorLog) { task.Stop(); } +UTEST(PeriodicTask, EnableDisable) { + SimpleTaskData simple; + + constexpr auto period = 3ms; + utils::PeriodicTask::Settings settings(period); + + constexpr Count n = 10; + + const auto start = std::chrono::steady_clock::now(); + utils::PeriodicTask task("task", period, simple.GetTaskFunction()); + EXPECT_TRUE(simple.WaitFor(period * n * kSlowRatio, [&simple]() { return simple.GetCount() > 5; })); + auto countBeforeStop = simple.GetCount(); + settings.enabled = false; + task.SetSettings(settings); + EXPECT_FALSE(simple.WaitFor(period * n * kSlowRatio, [&simple]() { return simple.GetCount() > 10; })); + EXPECT_TRUE(countBeforeStop == simple.GetCount()); + EXPECT_TRUE(task.IsRunning()); + settings.enabled = true; + task.SetSettings(settings); + EXPECT_TRUE(simple.WaitFor(period * n * kSlowRatio, [&simple,countBeforeStop]() { return simple.GetCount() > 10; })); + EXPECT_TRUE(task.IsRunning()); + auto countBeforeStop2 = simple.GetCount(); + settings.enabled = false; + task.SetSettings(settings); + EXPECT_FALSE(simple.WaitFor(period * n * kSlowRatio, [&simple]() { return simple.GetCount() > 20; })); + EXPECT_TRUE(countBeforeStop2 == simple.GetCount()); + EXPECT_TRUE(task.IsRunning()); + + task.ForceStepAsync(); + EXPECT_TRUE(simple.WaitFor(period * n * kSlowRatio, [&simple]() { return simple.GetCount() > 30; })); + + task.Stop(); + EXPECT_TRUE(!task.IsRunning()); +} + USERVER_NAMESPACE_END From d2916175f6dc3a8f4c318c09d46856814f980833 Mon Sep 17 00:00:00 2001 From: "alexander.klyuchev" Date: Sun, 6 Apr 2025 11:02:24 +0300 Subject: [PATCH 4/9] fix compile --- core/src/utils/periodic_task.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/core/src/utils/periodic_task.cpp b/core/src/utils/periodic_task.cpp index a5f16e26939f..3457cbf5bfda 100644 --- a/core/src/utils/periodic_task.cpp +++ b/core/src/utils/periodic_task.cpp @@ -22,8 +22,8 @@ namespace { auto TieSettings(const PeriodicTask::Settings& settings) { // Can't use Boost.Pfr, because Settings has custom constructors. - const auto& [f1, f2, f3, f4, f5, f6] = settings; - return std::tie(f1, f2, f3, f4, f5, f6); + const auto& [f1, f2, f3, f4, f5, f6, f7] = settings; + return std::tie(f1, f2, f3, f4, f5, f6, f7); } } // namespace @@ -121,6 +121,7 @@ void PeriodicTask::SetSettings(Settings settings) { void PeriodicTask::ForceStepAsync() { should_force_step_ = true; + auto writer = settings_.StartWrite(); if (!writer->enabled) { writer->enabled = true; writer.Commit(); From 3ea535784766c0a1386fe26f49992598c569534f Mon Sep 17 00:00:00 2001 From: "alexander.klyuchev" Date: Tue, 8 Apr 2025 08:51:33 +0300 Subject: [PATCH 5/9] review comments --- core/include/userver/utils/periodic_task.hpp | 4 ++++ core/src/utils/periodic_task.cpp | 19 +++++++------------ 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/core/include/userver/utils/periodic_task.hpp b/core/include/userver/utils/periodic_task.hpp index cf0991febc5e..02751c7e8ee5 100644 --- a/core/include/userver/utils/periodic_task.hpp +++ b/core/include/userver/utils/periodic_task.hpp @@ -126,6 +126,8 @@ class PeriodicTask final { engine::TaskProcessor* task_processor{nullptr}; /// @brief enabled allows to enable/disable timer during run + /// SynchronizeDebug does not run the iteration on enabled=false, because its logic is just to imitate the passing of time + /// ForceStepAsync runs the iteration even for enabled=false bool enabled{true}; }; @@ -173,6 +175,7 @@ class PeriodicTask final { /// @note If 'ForceStepAsync' is called multiple times while Step() is /// being executed, all events will be conflated (one extra Step() call will /// be executed). + /// Runs the iteration even for enabled=false void ForceStepAsync(); /// Force next DoStep() iteration. It is guaranteed that there is at least one @@ -184,6 +187,7 @@ class PeriodicTask final { /// @returns true if task was successfully executed. /// @note On concurrent invocations, the task is guaranteed to be invoked /// serially, one time after another. + /// Does not run the iteration on enabled=false, because its logic is just to imitate the passing of time bool SynchronizeDebug(bool preserve_span = false); /// Skip Step() calls from loop until ResumeDebug() is called. If DoStep() diff --git a/core/src/utils/periodic_task.cpp b/core/src/utils/periodic_task.cpp index 3457cbf5bfda..62961ce6933d 100644 --- a/core/src/utils/periodic_task.cpp +++ b/core/src/utils/periodic_task.cpp @@ -119,13 +119,7 @@ void PeriodicTask::SetSettings(Settings settings) { } void PeriodicTask::ForceStepAsync() { - should_force_step_ = true; - auto writer = settings_.StartWrite(); - if (!writer->enabled) { - writer->enabled = true; - writer.Commit(); - } changed_event_.Send(); } @@ -152,11 +146,11 @@ void PeriodicTask::Run() { const auto before = std::chrono::steady_clock::now(); bool no_exception = true; - const auto settings = settings_.Read(); - bool taskEnabled = settings->enabled; - if (!std::exchange(skip_step, false) && taskEnabled) { + bool task_enabled = settings->enabled; + if (!std::exchange(skip_step, false) && task_enabled) { no_exception = Step(); } + const auto settings = settings_.Read(); auto period = settings->period; const auto exception_period = settings->exception_period.value_or(period); @@ -170,14 +164,15 @@ void PeriodicTask::Run() { start = std::chrono::steady_clock::now(); } - while ((!taskEnabled && changed_event_.WaitForEvent()) || (taskEnabled && changed_event_.WaitForEventUntil(start + MutatePeriod(period)))) { + engine::Deadline dealine; + while (deadline = task_enabled ? start + MutatePeriod(period) : Deadline{}, WaitForEvent(deadline)) { if (should_force_step_.exchange(false)) { break; } // The config variable value has been changed, reload const auto settings = settings_.Read(); - taskEnabled = settings->enabled; - if(!taskEnabled) { + task_enabled = settings->enabled; + if(!task_enabled) { break; } From 0235bfc8ae161cd7efd031cc81097495760ad773 Mon Sep 17 00:00:00 2001 From: "alexander.klyuchev" Date: Tue, 8 Apr 2025 09:08:00 +0300 Subject: [PATCH 6/9] fix compile --- core/src/utils/periodic_task.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/core/src/utils/periodic_task.cpp b/core/src/utils/periodic_task.cpp index 62961ce6933d..7cc307444d37 100644 --- a/core/src/utils/periodic_task.cpp +++ b/core/src/utils/periodic_task.cpp @@ -135,8 +135,10 @@ bool PeriodicTask::IsRunning() const { return task_.IsValid(); } void PeriodicTask::Run() { bool skip_step = false; + bool task_enabled = true; { auto settings = settings_.Read(); + task_enabled = settings->enabled; if (!(settings->flags & Flags::kNow)) { skip_step = true; } @@ -146,12 +148,12 @@ void PeriodicTask::Run() { const auto before = std::chrono::steady_clock::now(); bool no_exception = true; - bool task_enabled = settings->enabled; if (!std::exchange(skip_step, false) && task_enabled) { no_exception = Step(); } - const auto settings = settings_.Read(); + auto settings = settings_.Read(); + task_enabled = settings->enabled; auto period = settings->period; const auto exception_period = settings->exception_period.value_or(period); @@ -165,7 +167,7 @@ void PeriodicTask::Run() { } engine::Deadline dealine; - while (deadline = task_enabled ? start + MutatePeriod(period) : Deadline{}, WaitForEvent(deadline)) { + while (deadline = task_enabled ? start + MutatePeriod(period) : engine::Deadline{}, WaitForEvent(deadline)) { if (should_force_step_.exchange(false)) { break; } From 1f679d26a665391da727a4d7e62dafbee0c64529 Mon Sep 17 00:00:00 2001 From: "alexander.klyuchev" Date: Tue, 8 Apr 2025 09:58:12 +0300 Subject: [PATCH 7/9] fix compile --- core/src/utils/periodic_task.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/utils/periodic_task.cpp b/core/src/utils/periodic_task.cpp index 7cc307444d37..5e9f7fe6e15a 100644 --- a/core/src/utils/periodic_task.cpp +++ b/core/src/utils/periodic_task.cpp @@ -166,7 +166,7 @@ void PeriodicTask::Run() { start = std::chrono::steady_clock::now(); } - engine::Deadline dealine; + engine::Deadline deadline; while (deadline = task_enabled ? start + MutatePeriod(period) : engine::Deadline{}, WaitForEvent(deadline)) { if (should_force_step_.exchange(false)) { break; From d255a55ed854dc756acbfa4800c40e11b5b8a1ab Mon Sep 17 00:00:00 2001 From: "alexander.klyuchev" Date: Wed, 9 Apr 2025 16:04:29 +0300 Subject: [PATCH 8/9] fix compile --- core/src/utils/periodic_task.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/core/src/utils/periodic_task.cpp b/core/src/utils/periodic_task.cpp index 5e9f7fe6e15a..9422d144ad30 100644 --- a/core/src/utils/periodic_task.cpp +++ b/core/src/utils/periodic_task.cpp @@ -167,15 +167,16 @@ void PeriodicTask::Run() { } engine::Deadline deadline; - while (deadline = task_enabled ? start + MutatePeriod(period) : engine::Deadline{}, WaitForEvent(deadline)) { + while (deadline = task_enabled ? engine::Deadline::FromTimePoint(start + MutatePeriod(period) : engine::Deadline{}, changed_event_.WaitForEventUntil(deadline)) { if (should_force_step_.exchange(false)) { break; } // The config variable value has been changed, reload const auto settings = settings_.Read(); - task_enabled = settings->enabled; - if(!task_enabled) { - break; + if(task_enabled != settings->enabled) + { + task_enabled = settings->enabled; + break; } period = settings->period; From 0ddcd62a02fa60ca99439ab8d95feec90ca3afca Mon Sep 17 00:00:00 2001 From: "alexander.klyuchev" Date: Wed, 9 Apr 2025 16:17:16 +0300 Subject: [PATCH 9/9] fix compile --- core/src/utils/periodic_task.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/utils/periodic_task.cpp b/core/src/utils/periodic_task.cpp index 9422d144ad30..c7b2e2a863a4 100644 --- a/core/src/utils/periodic_task.cpp +++ b/core/src/utils/periodic_task.cpp @@ -167,7 +167,7 @@ void PeriodicTask::Run() { } engine::Deadline deadline; - while (deadline = task_enabled ? engine::Deadline::FromTimePoint(start + MutatePeriod(period) : engine::Deadline{}, changed_event_.WaitForEventUntil(deadline)) { + while (deadline = task_enabled ? engine::Deadline::FromTimePoint(start + MutatePeriod(period)) : engine::Deadline{}, changed_event_.WaitForEventUntil(deadline)) { if (should_force_step_.exchange(false)) { break; }