From fb8eb539fca224803014d43c220abbd6e704a962 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Mon, 12 Aug 2024 15:51:14 +0200 Subject: [PATCH] libstore: move respect-timeoutiness to goal method this is useless to do on the face of it, but it'll make it easier to convert the entire output handling to use async io and promises soon Change-Id: I2d1eb62c4bbf8f57bd558b9599c08710a389b1a8 --- src/libstore/build/derivation-goal.cc | 2 +- .../build/drv-output-substitution-goal.cc | 2 +- src/libstore/build/goal.hh | 5 +++++ src/libstore/build/local-derivation-goal.cc | 2 +- src/libstore/build/local-derivation-goal.hh | 4 ++++ src/libstore/build/substitution-goal.cc | 2 +- src/libstore/build/worker.cc | 19 ++++++++++--------- src/libstore/build/worker.hh | 3 +-- 8 files changed, 24 insertions(+), 15 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index cc445a0a4..2781929e7 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -1213,7 +1213,7 @@ HookReply DerivationGoal::tryBuildHook(bool inBuildSlot) fds.insert(hook->fromHook.readSide.get()); fds.insert(hook->builderOut.readSide.get()); builderOutFD = &hook->builderOut.readSide; - worker.childStarted(shared_from_this(), fds, false, false); + worker.childStarted(shared_from_this(), fds, false); return rpAccept; } diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index 087673be7..aea055624 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -75,7 +75,7 @@ Goal::WorkResult DrvOutputSubstitutionGoal::tryNext(bool inBuildSlot) return sub->queryRealisation(id); }); - worker.childStarted(shared_from_this(), {downloadState->outPipe.readSide.get()}, true, false); + worker.childStarted(shared_from_this(), {downloadState->outPipe.readSide.get()}, true); state = &DrvOutputSubstitutionGoal::realisationFetched; return StillAlive{}; diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh index fd7534b0a..114abda41 100644 --- a/src/libstore/build/goal.hh +++ b/src/libstore/build/goal.hh @@ -160,6 +160,11 @@ public: { } + virtual bool respectsTimeouts() + { + return false; + } + void trace(std::string_view s); std::string getName() const diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 116fa0ae5..afb9524ef 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -726,7 +726,7 @@ void LocalDerivationGoal::startBuilder() /* parent */ pid.setSeparatePG(true); - worker.childStarted(shared_from_this(), {builderOutPTY.get()}, true, true); + worker.childStarted(shared_from_this(), {builderOutPTY.get()}, true); /* Check if setting up the build environment failed. */ std::vector msgs; diff --git a/src/libstore/build/local-derivation-goal.hh b/src/libstore/build/local-derivation-goal.hh index 128e6fd36..9b2391256 100644 --- a/src/libstore/build/local-derivation-goal.hh +++ b/src/libstore/build/local-derivation-goal.hh @@ -357,6 +357,10 @@ protected: return false; } + virtual bool respectsTimeouts() override + { + return true; + } }; } diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc index cacf3618f..281ac216c 100644 --- a/src/libstore/build/substitution-goal.cc +++ b/src/libstore/build/substitution-goal.cc @@ -221,7 +221,7 @@ Goal::WorkResult PathSubstitutionGoal::tryToRun(bool inBuildSlot) } }); - worker.childStarted(shared_from_this(), {outPipe.readSide.get()}, true, false); + worker.childStarted(shared_from_this(), {outPipe.readSide.get()}, true); state = &PathSubstitutionGoal::finished; return StillAlive{}; diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 411525d94..325de3073 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -233,7 +233,7 @@ void Worker::wakeUp(GoalPtr goal) void Worker::childStarted(GoalPtr goal, const std::set & fds, - bool inBuildSlot, bool respectTimeouts) + bool inBuildSlot) { Child child; child.goal = goal; @@ -241,7 +241,6 @@ void Worker::childStarted(GoalPtr goal, const std::set & fds, child.fds = fds; child.timeStarted = child.lastOutput = steady_time_point::clock::now(); child.inBuildSlot = inBuildSlot; - child.respectTimeouts = respectTimeouts; children.emplace_back(child); if (inBuildSlot) { switch (goal->jobCategory()) { @@ -427,11 +426,13 @@ void Worker::waitForInput() // Periodicallty wake up to see if we need to run the garbage collector. nearest = before + std::chrono::seconds(10); for (auto & i : children) { - if (!i.respectTimeouts) continue; - if (0 != settings.maxSilentTime) - nearest = std::min(nearest, i.lastOutput + std::chrono::seconds(settings.maxSilentTime)); - if (0 != settings.buildTimeout) - nearest = std::min(nearest, i.timeStarted + std::chrono::seconds(settings.buildTimeout)); + if (auto goal = i.goal.lock()) { + if (!goal->respectsTimeouts()) continue; + if (0 != settings.maxSilentTime) + nearest = std::min(nearest, i.lastOutput + std::chrono::seconds(settings.maxSilentTime)); + if (0 != settings.buildTimeout) + nearest = std::min(nearest, i.timeStarted + std::chrono::seconds(settings.buildTimeout)); + } } if (nearest != steady_time_point::max()) { timeout = std::max(1L, (long) std::chrono::duration_cast(nearest - before).count()); @@ -484,7 +485,7 @@ void Worker::waitForInput() if (!goal->exitCode.has_value() && 0 != settings.maxSilentTime && - j->respectTimeouts && + goal->respectsTimeouts() && after - j->lastOutput >= std::chrono::seconds(settings.maxSilentTime)) { handleWorkResult( @@ -500,7 +501,7 @@ void Worker::waitForInput() else if (!goal->exitCode.has_value() && 0 != settings.buildTimeout && - j->respectTimeouts && + goal->respectsTimeouts() && after - j->timeStarted >= std::chrono::seconds(settings.buildTimeout)) { handleWorkResult( diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh index 5f80880a9..406bf205d 100644 --- a/src/libstore/build/worker.hh +++ b/src/libstore/build/worker.hh @@ -29,7 +29,6 @@ struct Child WeakGoalPtr goal; Goal * goal2; // ugly hackery std::set fds; - bool respectTimeouts; bool inBuildSlot; /** * Time we last got output on stdout/stderr @@ -234,7 +233,7 @@ public: * the process counts towards the jobs limit. */ void childStarted(GoalPtr goal, const std::set & fds, - bool inBuildSlot, bool respectTimeouts); + bool inBuildSlot); /** * Unregisters a running child process.