Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions core/include/userver/utils/periodic_task.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
27 changes: 20 additions & 7 deletions core/src/utils/periodic_task.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -98,6 +98,7 @@ void PeriodicTask::Stop() noexcept {
}

void PeriodicTask::SetSettings(Settings settings) {

bool should_notify_task{};
{
auto writer = settings_.StartWrite();
Expand All @@ -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();
}
Expand All @@ -118,7 +119,13 @@ 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();
}

Expand All @@ -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);

Expand All @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting deadline + continue should be more straightforward and closer to existing code logic below

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you mean?

}

period = settings->period;
const auto exception_period = settings->exception_period.value_or(period);
if (!no_exception) period = exception_period;
Expand Down
35 changes: 35 additions & 0 deletions core/src/utils/periodic_task_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -418,4 +418,39 @@ UTEST_F(PeriodicTaskLog, ErrorLog) {
task.Stop();
}

UTEST(PeriodicTask, EnableDisable) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a test that checks the task does not keep running periodically if ForceStepAsync is run while enabled=false

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test will also check that ForceStepAsync ignores enabled

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a lot to decipher in this test's logic - split it into some steps with empty lines and comment on each step

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a test that checks that if enabled=false during Start (even if enabled=true in PeriodicTask constructor), then the first iteration won't happen immediately and won't happen after period

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a test that checks that SynchronizeDebug respects enabled

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a test that after enabled=false + enabled=on the task does not always start an iteration instantly, but awaits the configured interval since the last time it was run

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
Loading