From 30ab41288c25f9d65fdffc69783f6acfc530c9ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Malinowski?= Date: Thu, 27 Jul 2017 14:32:43 +0200 Subject: [PATCH 1/3] test scenerios where work scheduling fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rafał Malinowski --- src/work/WorkTests.cpp | 150 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 150 insertions(+) diff --git a/src/work/WorkTests.cpp b/src/work/WorkTests.cpp index 007bf071f8..557a3fd537 100644 --- a/src/work/WorkTests.cpp +++ b/src/work/WorkTests.cpp @@ -96,3 +96,153 @@ TEST_CASE("work steps", "[work]") clock.crank(); } } + +class FailingWork : public Work +{ + public: + FailingWork(Application& app, WorkParent& parent, std::string const& uniqueName) + : Work(app, parent, uniqueName, 0) + { + } + + virtual void + onRun() override + { + scheduleFailure(); + } +}; + +class WorkCountingOnFailureRaise : public Work +{ + public: + int mCount {0}; + + WorkCountingOnFailureRaise(Application& app, WorkParent& parent) + : Work(app, parent, std::string("counting-on-complete"), 0) + { + } + + virtual void + onRun() override + { + mCount++; + Work::onRun(); + } + + virtual void + onFailureRaise() override + { + mCount++; + Work::onFailureRaise(); + } +}; + +TEST_CASE("subwork items fail at the same time", "[work]") +{ + VirtualClock clock; + auto const& cfg = getTestConfig(); + auto app = Application::create(clock, cfg); + auto& wm = app->getWorkManager(); + auto w = wm.addWork(); + w->addWork("work-1"); + w->addWork("work-2"); + wm.advanceChildren(); + while (!wm.allChildrenDone()) + { + clock.crank(); + } + + REQUIRE(w->mCount == 2); +} + +class WorkDoNothing : public Work +{ + public: + WorkDoNothing(Application& app, WorkParent& parent, std::string const& uniqueName) + : Work(app, parent, uniqueName, 0) + { + } + + virtual void + onRun() override + { + } + + void forceSuccess() + { + callComplete()({}); + } +}; + +class WorkWith2Subworks : public Work +{ + public: + WorkWith2Subworks(Application& app, WorkParent& parent, std::string const& uniqueName) + : Work(app, parent, uniqueName, 0) + { + } + + virtual void + onReset() override + { + clearChildren(); + mFirstSubwork.reset(); + mSecondSubwork.reset(); + + mFirstSubwork = + addWork("first-subwork-of-" + getUniqueName()); + } + + virtual Work::State + onSuccess() override + { + CLOG(DEBUG, "History") << "onSuccess() for " << getUniqueName(); + + if (mSecondSubwork) + { + mCalledSuccessWithPendingSubwork = mCalledSuccessWithPendingSubwork || mSecondSubwork->getState() == WORK_PENDING; + return WORK_SUCCESS; + } + + mSecondSubwork = addWork("second-subwork-of-" + getUniqueName()); + return WORK_PENDING; + } + + std::shared_ptr mFirstSubwork; + std::shared_ptr mSecondSubwork; + bool mCalledSuccessWithPendingSubwork {false}; +}; + +TEST_CASE("sub-subwork items succed at the same time", "[work]") +{ + VirtualClock clock; + auto const& cfg = getTestConfig(); + auto app = Application::create(clock, cfg); + auto& wm = app->getWorkManager(); + auto w = wm.addWork("parent-of-many"); + auto work1 = w->addWork("work-1"); + auto work2 = w->addWork("work-2"); + auto work3 = w->addWork("work-3"); + + auto i = 0; + + wm.advanceChildren(); + while (!wm.allChildrenDone()) + { + clock.crank(false); + + switch (i++) + { + case 1: + work2->forceSuccess(); + work1->mFirstSubwork->forceSuccess(); + work3->forceSuccess(); + break; + case 2: + work1->mSecondSubwork->forceSuccess(); + break; + } + } + + REQUIRE(work1->mCalledSuccessWithPendingSubwork); +} From 476671288d8377899b25ff1d017ab6eb774b736e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Malinowski?= Date: Thu, 27 Jul 2017 14:49:19 +0200 Subject: [PATCH 2/3] only allow one work schedule at a time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rafał Malinowski --- src/historywork/GetAndUnzipRemoteFileWork.cpp | 5 ----- src/work/Work.cpp | 22 +++++++++++++++++++ src/work/Work.h | 1 + src/work/WorkTests.cpp | 4 ++-- 4 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/historywork/GetAndUnzipRemoteFileWork.cpp b/src/historywork/GetAndUnzipRemoteFileWork.cpp index 26d308a5d4..50ee5a31e2 100644 --- a/src/historywork/GetAndUnzipRemoteFileWork.cpp +++ b/src/historywork/GetAndUnzipRemoteFileWork.cpp @@ -57,11 +57,6 @@ GetAndUnzipRemoteFileWork::onSuccess() { if (mGunzipFileWork) { - if (mGunzipFileWork->getState() != WORK_SUCCESS) - { - return WORK_PENDING; - } - if (!fs::exists(mFt.localPath_nogz())) { CLOG(ERROR, "History") << "Downloading and unzipping " diff --git a/src/work/Work.cpp b/src/work/Work.cpp index 1b42c1722b..40039e7d79 100644 --- a/src/work/Work.cpp +++ b/src/work/Work.cpp @@ -8,6 +8,7 @@ #include "util/Logging.h" #include "util/Math.h" #include "util/make_unique.h" +#include "work/WorkManager.h" #include "work/WorkParent.h" #include "medida/meter.h" @@ -137,15 +138,22 @@ Work::callComplete() void Work::scheduleRun() { + if (mScheduled) + { + return; + } + std::weak_ptr weak( std::static_pointer_cast(shared_from_this())); CLOG(DEBUG, "Work") << "scheduling run of " << getUniqueName(); + mScheduled = true; mApp.getClock().getIOService().post([weak]() { auto self = weak.lock(); if (!self) { return; } + self->mScheduled = false; self->run(); }); } @@ -153,15 +161,22 @@ Work::scheduleRun() void Work::scheduleComplete(CompleteResult result) { + if (mScheduled) + { + return; + } + std::weak_ptr weak( std::static_pointer_cast(shared_from_this())); CLOG(DEBUG, "Work") << "scheduling completion of " << getUniqueName(); + mScheduled = true; mApp.getClock().getIOService().post([weak, result]() { auto self = weak.lock(); if (!self) { return; } + self->mScheduled = false; self->complete(result); }); } @@ -169,6 +184,11 @@ Work::scheduleComplete(CompleteResult result) void Work::scheduleRetry() { + if (mScheduled) + { + return; + } + if (getState() != WORK_FAILURE_RETRY) { std::string msg = fmt::format("retrying {} in state {}", @@ -190,6 +210,7 @@ Work::scheduleRetry() << "Scheduling retry #" << (mRetries + 1) << "/" << mMaxRetries << " in " << std::chrono::duration_cast(t).count() << " sec, for " << getUniqueName(); + mScheduled = true; mRetryTimer->async_wait( [weak]() { auto self = weak.lock(); @@ -197,6 +218,7 @@ Work::scheduleRetry() { return; } + self->mScheduled = false; self->mRetries++; self->reset(); self->advance(); diff --git a/src/work/Work.h b/src/work/Work.h index be04f8b24d..a341a711b1 100644 --- a/src/work/Work.h +++ b/src/work/Work.h @@ -103,6 +103,7 @@ class Work : public WorkParent size_t mMaxRetries{RETRY_A_FEW}; size_t mRetries{0}; State mState{WORK_PENDING}; + bool mScheduled {false}; std::unique_ptr mRetryTimer; diff --git a/src/work/WorkTests.cpp b/src/work/WorkTests.cpp index 557a3fd537..ddc8dd2254 100644 --- a/src/work/WorkTests.cpp +++ b/src/work/WorkTests.cpp @@ -152,7 +152,7 @@ TEST_CASE("subwork items fail at the same time", "[work]") clock.crank(); } - REQUIRE(w->mCount == 2); + REQUIRE(w->mCount == 1); } class WorkDoNothing : public Work @@ -244,5 +244,5 @@ TEST_CASE("sub-subwork items succed at the same time", "[work]") } } - REQUIRE(work1->mCalledSuccessWithPendingSubwork); + REQUIRE(!work1->mCalledSuccessWithPendingSubwork); } From f6535d8b247b0b58823b5a73f0b9748d79e79952 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Malinowski?= Date: Tue, 1 Aug 2017 13:34:34 +0200 Subject: [PATCH 3/3] clang-format on WorkTests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rafał Malinowski --- src/work/WorkTests.cpp | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/work/WorkTests.cpp b/src/work/WorkTests.cpp index ddc8dd2254..f17eb2121e 100644 --- a/src/work/WorkTests.cpp +++ b/src/work/WorkTests.cpp @@ -100,7 +100,8 @@ TEST_CASE("work steps", "[work]") class FailingWork : public Work { public: - FailingWork(Application& app, WorkParent& parent, std::string const& uniqueName) + FailingWork(Application& app, WorkParent& parent, + std::string const& uniqueName) : Work(app, parent, uniqueName, 0) { } @@ -115,7 +116,7 @@ class FailingWork : public Work class WorkCountingOnFailureRaise : public Work { public: - int mCount {0}; + int mCount{0}; WorkCountingOnFailureRaise(Application& app, WorkParent& parent) : Work(app, parent, std::string("counting-on-complete"), 0) @@ -158,7 +159,8 @@ TEST_CASE("subwork items fail at the same time", "[work]") class WorkDoNothing : public Work { public: - WorkDoNothing(Application& app, WorkParent& parent, std::string const& uniqueName) + WorkDoNothing(Application& app, WorkParent& parent, + std::string const& uniqueName) : Work(app, parent, uniqueName, 0) { } @@ -168,7 +170,8 @@ class WorkDoNothing : public Work { } - void forceSuccess() + void + forceSuccess() { callComplete()({}); } @@ -177,7 +180,8 @@ class WorkDoNothing : public Work class WorkWith2Subworks : public Work { public: - WorkWith2Subworks(Application& app, WorkParent& parent, std::string const& uniqueName) + WorkWith2Subworks(Application& app, WorkParent& parent, + std::string const& uniqueName) : Work(app, parent, uniqueName, 0) { } @@ -200,17 +204,20 @@ class WorkWith2Subworks : public Work if (mSecondSubwork) { - mCalledSuccessWithPendingSubwork = mCalledSuccessWithPendingSubwork || mSecondSubwork->getState() == WORK_PENDING; + mCalledSuccessWithPendingSubwork = + mCalledSuccessWithPendingSubwork || + mSecondSubwork->getState() == WORK_PENDING; return WORK_SUCCESS; } - mSecondSubwork = addWork("second-subwork-of-" + getUniqueName()); + mSecondSubwork = + addWork("second-subwork-of-" + getUniqueName()); return WORK_PENDING; } std::shared_ptr mFirstSubwork; std::shared_ptr mSecondSubwork; - bool mCalledSuccessWithPendingSubwork {false}; + bool mCalledSuccessWithPendingSubwork{false}; }; TEST_CASE("sub-subwork items succed at the same time", "[work]")