diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index a3dd88ca2..291504a0f 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -129,9 +129,9 @@ Goal::Finished DerivationGoal::timedOut(Error && ex) } -Goal::WorkResult DerivationGoal::work() +Goal::WorkResult DerivationGoal::work(bool inBuildSlot) { - return (this->*state)(); + return (this->*state)(inBuildSlot); } void DerivationGoal::addWantedOutputs(const OutputsSpec & outputs) @@ -155,7 +155,7 @@ void DerivationGoal::addWantedOutputs(const OutputsSpec & outputs) } -Goal::WorkResult DerivationGoal::getDerivation() +Goal::WorkResult DerivationGoal::getDerivation(bool inBuildSlot) { trace("init"); @@ -163,7 +163,7 @@ Goal::WorkResult DerivationGoal::getDerivation() exists. If it doesn't, it may be created through a substitute. */ if (buildMode == bmNormal && worker.evalStore.isValidPath(drvPath)) { - return loadDerivation(); + return loadDerivation(inBuildSlot); } @@ -172,7 +172,7 @@ Goal::WorkResult DerivationGoal::getDerivation() } -Goal::WorkResult DerivationGoal::loadDerivation() +Goal::WorkResult DerivationGoal::loadDerivation(bool inBuildSlot) { trace("loading derivation"); @@ -199,11 +199,11 @@ Goal::WorkResult DerivationGoal::loadDerivation() } assert(drv); - return haveDerivation(); + return haveDerivation(inBuildSlot); } -Goal::WorkResult DerivationGoal::haveDerivation() +Goal::WorkResult DerivationGoal::haveDerivation(bool inBuildSlot) { trace("have derivation"); @@ -231,7 +231,7 @@ Goal::WorkResult DerivationGoal::haveDerivation() }); } - return gaveUpOnSubstitution(); + return gaveUpOnSubstitution(inBuildSlot); } for (auto & i : drv->outputsAndOptPaths(worker.store)) @@ -280,14 +280,14 @@ Goal::WorkResult DerivationGoal::haveDerivation() } if (result.goals.empty()) { /* to prevent hang (no wake-up event) */ - return outputsSubstitutionTried(); + return outputsSubstitutionTried(inBuildSlot); } else { state = &DerivationGoal::outputsSubstitutionTried; return result; } } -Goal::WorkResult DerivationGoal::outputsSubstitutionTried() +Goal::WorkResult DerivationGoal::outputsSubstitutionTried(bool inBuildSlot) { trace("all outputs substituted (maybe)"); @@ -330,7 +330,7 @@ Goal::WorkResult DerivationGoal::outputsSubstitutionTried() if (needRestart == NeedRestartForMoreOutputs::OutputsAddedDoNeed) { needRestart = NeedRestartForMoreOutputs::OutputsUnmodifedDontNeed; - return haveDerivation(); + return haveDerivation(inBuildSlot); } auto [allValid, validOutputs] = checkPathValidity(); @@ -346,13 +346,13 @@ Goal::WorkResult DerivationGoal::outputsSubstitutionTried() worker.store.printStorePath(drvPath)); /* Nothing to wait for; tail call */ - return gaveUpOnSubstitution(); + return gaveUpOnSubstitution(inBuildSlot); } /* At least one of the output paths could not be produced using a substitute. So we have to build instead. */ -Goal::WorkResult DerivationGoal::gaveUpOnSubstitution() +Goal::WorkResult DerivationGoal::gaveUpOnSubstitution(bool inBuildSlot) { WaitForGoals result; @@ -416,7 +416,7 @@ Goal::WorkResult DerivationGoal::gaveUpOnSubstitution() } if (result.goals.empty()) {/* to prevent hang (no wake-up event) */ - return inputsRealised(); + return inputsRealised(inBuildSlot); } else { state = &DerivationGoal::inputsRealised; return result; @@ -487,7 +487,7 @@ Goal::WorkResult DerivationGoal::repairClosure() } -Goal::WorkResult DerivationGoal::closureRepaired() +Goal::WorkResult DerivationGoal::closureRepaired(bool inBuildSlot) { trace("closure repaired"); if (nrFailed > 0) @@ -497,7 +497,7 @@ Goal::WorkResult DerivationGoal::closureRepaired() } -Goal::WorkResult DerivationGoal::inputsRealised() +Goal::WorkResult DerivationGoal::inputsRealised(bool inBuildSlot) { trace("all inputs realised"); @@ -511,7 +511,7 @@ Goal::WorkResult DerivationGoal::inputsRealised() if (retrySubstitution == RetrySubstitution::YesNeed) { retrySubstitution = RetrySubstitution::AlreadyRetried; - return haveDerivation(); + return haveDerivation(inBuildSlot); } /* Gather information necessary for computing the closure and/or @@ -659,7 +659,7 @@ Goal::WorkResult DerivationGoal::started() return StillAlive{}; } -Goal::WorkResult DerivationGoal::tryToBuild() +Goal::WorkResult DerivationGoal::tryToBuild(bool inBuildSlot) { trace("trying to build"); @@ -731,7 +731,7 @@ Goal::WorkResult DerivationGoal::tryToBuild() && settings.maxBuildJobs.get() != 0; if (!buildLocally) { - switch (tryBuildHook()) { + switch (tryBuildHook(inBuildSlot)) { case rpAccept: /* Yes, it has started doing so. Wait until we get EOF from the hook. */ @@ -759,7 +759,7 @@ Goal::WorkResult DerivationGoal::tryToBuild() return ContinueImmediately{}; } -Goal::WorkResult DerivationGoal::tryLocalBuild() { +Goal::WorkResult DerivationGoal::tryLocalBuild(bool inBuildSlot) { throw Error( "unable to build with a primary store that isn't a local store; " "either pass a different '--store' or enable remote builds." @@ -917,7 +917,7 @@ void runPostBuildHook( proc.getStdout()->drainInto(sink); } -Goal::WorkResult DerivationGoal::buildDone() +Goal::WorkResult DerivationGoal::buildDone(bool inBuildSlot) { trace("build done"); @@ -1037,7 +1037,7 @@ Goal::WorkResult DerivationGoal::buildDone() } } -Goal::WorkResult DerivationGoal::resolvedFinished() +Goal::WorkResult DerivationGoal::resolvedFinished(bool inBuildSlot) { trace("resolved derivation finished"); @@ -1108,7 +1108,7 @@ Goal::WorkResult DerivationGoal::resolvedFinished() return done(status, std::move(builtOutputs)); } -HookReply DerivationGoal::tryBuildHook() +HookReply DerivationGoal::tryBuildHook(bool inBuildSlot) { if (!worker.hook.available || !useDerivation) return rpDecline; @@ -1120,7 +1120,7 @@ HookReply DerivationGoal::tryBuildHook() /* Send the request to the hook. */ worker.hook.instance->sink << "try" - << (worker.getNrLocalBuilds() < settings.maxBuildJobs ? 1 : 0) + << (inBuildSlot ? 1 : 0) << drv->platform << worker.store.printStorePath(drvPath) << parsedDrv->getRequiredSystemFeatures(); diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index 257282308..250b437da 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -191,7 +191,7 @@ struct DerivationGoal : public Goal */ std::optional derivationType; - typedef WorkResult (DerivationGoal::*GoalState)(); + typedef WorkResult (DerivationGoal::*GoalState)(bool inBuildSlot); GoalState state; BuildMode buildMode; @@ -224,7 +224,7 @@ struct DerivationGoal : public Goal std::string key() override; - WorkResult work() override; + WorkResult work(bool inBuildSlot) override; /** * Add wanted outputs to an already existing derivation goal. @@ -234,23 +234,23 @@ struct DerivationGoal : public Goal /** * The states. */ - WorkResult getDerivation(); - WorkResult loadDerivation(); - WorkResult haveDerivation(); - WorkResult outputsSubstitutionTried(); - WorkResult gaveUpOnSubstitution(); - WorkResult closureRepaired(); - WorkResult inputsRealised(); - WorkResult tryToBuild(); - virtual WorkResult tryLocalBuild(); - WorkResult buildDone(); + WorkResult getDerivation(bool inBuildSlot); + WorkResult loadDerivation(bool inBuildSlot); + WorkResult haveDerivation(bool inBuildSlot); + WorkResult outputsSubstitutionTried(bool inBuildSlot); + WorkResult gaveUpOnSubstitution(bool inBuildSlot); + WorkResult closureRepaired(bool inBuildSlot); + WorkResult inputsRealised(bool inBuildSlot); + WorkResult tryToBuild(bool inBuildSlot); + virtual WorkResult tryLocalBuild(bool inBuildSlot); + WorkResult buildDone(bool inBuildSlot); - WorkResult resolvedFinished(); + WorkResult resolvedFinished(bool inBuildSlot); /** * Is the build hook willing to perform the build? */ - HookReply tryBuildHook(); + HookReply tryBuildHook(bool inBuildSlot); virtual int getChildStatus(); diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index b41dae5d6..087673be7 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -20,7 +20,7 @@ DrvOutputSubstitutionGoal::DrvOutputSubstitutionGoal( } -Goal::WorkResult DrvOutputSubstitutionGoal::init() +Goal::WorkResult DrvOutputSubstitutionGoal::init(bool inBuildSlot) { trace("init"); @@ -30,17 +30,14 @@ Goal::WorkResult DrvOutputSubstitutionGoal::init() } subs = settings.useSubstitutes ? getDefaultSubstituters() : std::list>(); - return tryNext(); + return tryNext(inBuildSlot); } -Goal::WorkResult DrvOutputSubstitutionGoal::tryNext() +Goal::WorkResult DrvOutputSubstitutionGoal::tryNext(bool inBuildSlot) { trace("trying next substituter"); - /* Make sure that we are allowed to start a substitution. Note that even - if maxSubstitutionJobs == 0, we still allow a substituter to run. This - prevents infinite waiting. */ - if (worker.runningSubstitutions >= std::max(1U, settings.maxSubstitutionJobs.get())) { + if (!inBuildSlot) { return WaitForSlot{}; } @@ -84,7 +81,7 @@ Goal::WorkResult DrvOutputSubstitutionGoal::tryNext() return StillAlive{}; } -Goal::WorkResult DrvOutputSubstitutionGoal::realisationFetched() +Goal::WorkResult DrvOutputSubstitutionGoal::realisationFetched(bool inBuildSlot) { worker.childTerminated(this); maintainRunningSubstitutions.reset(); @@ -97,7 +94,7 @@ Goal::WorkResult DrvOutputSubstitutionGoal::realisationFetched() } if (!outputInfo) { - return tryNext(); + return tryNext(inBuildSlot); } WaitForGoals result; @@ -114,7 +111,7 @@ Goal::WorkResult DrvOutputSubstitutionGoal::realisationFetched() worker.store.printStorePath(localOutputInfo->outPath), worker.store.printStorePath(depPath) ); - return tryNext(); + return tryNext(inBuildSlot); } result.goals.insert(worker.makeDrvOutputSubstitutionGoal(depId)); } @@ -123,14 +120,14 @@ Goal::WorkResult DrvOutputSubstitutionGoal::realisationFetched() result.goals.insert(worker.makePathSubstitutionGoal(outputInfo->outPath)); if (result.goals.empty()) { - return outPathValid(); + return outPathValid(inBuildSlot); } else { state = &DrvOutputSubstitutionGoal::outPathValid; return result; } } -Goal::WorkResult DrvOutputSubstitutionGoal::outPathValid() +Goal::WorkResult DrvOutputSubstitutionGoal::outPathValid(bool inBuildSlot) { assert(outputInfo); trace("output path substituted"); @@ -159,9 +156,9 @@ std::string DrvOutputSubstitutionGoal::key() return "a$" + std::string(id.to_string()); } -Goal::WorkResult DrvOutputSubstitutionGoal::work() +Goal::WorkResult DrvOutputSubstitutionGoal::work(bool inBuildSlot) { - return (this->*state)(); + return (this->*state)(inBuildSlot); } diff --git a/src/libstore/build/drv-output-substitution-goal.hh b/src/libstore/build/drv-output-substitution-goal.hh index a28347f15..a660a4f3e 100644 --- a/src/libstore/build/drv-output-substitution-goal.hh +++ b/src/libstore/build/drv-output-substitution-goal.hh @@ -58,20 +58,20 @@ class DrvOutputSubstitutionGoal : public Goal { public: DrvOutputSubstitutionGoal(const DrvOutput& id, Worker & worker, RepairFlag repair = NoRepair, std::optional ca = std::nullopt); - typedef WorkResult (DrvOutputSubstitutionGoal::*GoalState)(); + typedef WorkResult (DrvOutputSubstitutionGoal::*GoalState)(bool inBuildSlot); GoalState state; - WorkResult init(); - WorkResult tryNext(); - WorkResult realisationFetched(); - WorkResult outPathValid(); + WorkResult init(bool inBuildSlot); + WorkResult tryNext(bool inBuildSlot); + WorkResult realisationFetched(bool inBuildSlot); + WorkResult outPathValid(bool inBuildSlot); WorkResult finished(); Finished timedOut(Error && ex) override { abort(); }; std::string key() override; - WorkResult work() override; + WorkResult work(bool inBuildSlot) override; JobCategory jobCategory() const override { return JobCategory::Substitution; diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh index 5d7bb72b8..fd7534b0a 100644 --- a/src/libstore/build/goal.hh +++ b/src/libstore/build/goal.hh @@ -147,7 +147,7 @@ public: trace("goal destroyed"); } - virtual WorkResult work() = 0; + virtual WorkResult work(bool inBuildSlot) = 0; virtual void waiteeDone(GoalPtr waitee) { } diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 8e184b9bc..841d1f2f6 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -150,14 +150,13 @@ void LocalDerivationGoal::killSandbox(bool getStats) } -Goal::WorkResult LocalDerivationGoal::tryLocalBuild() +Goal::WorkResult LocalDerivationGoal::tryLocalBuild(bool inBuildSlot) { #if __APPLE__ additionalSandboxProfile = parsedDrv->getStringAttr("__sandboxProfile").value_or(""); #endif - unsigned int curBuilds = worker.getNrLocalBuilds(); - if (curBuilds >= settings.maxBuildJobs) { + if (!inBuildSlot) { state = &DerivationGoal::tryToBuild; outputLocks.unlock(); return WaitForSlot{}; diff --git a/src/libstore/build/local-derivation-goal.hh b/src/libstore/build/local-derivation-goal.hh index e87f2c696..7617760a5 100644 --- a/src/libstore/build/local-derivation-goal.hh +++ b/src/libstore/build/local-derivation-goal.hh @@ -211,7 +211,7 @@ struct LocalDerivationGoal : public DerivationGoal /** * The additional states. */ - WorkResult tryLocalBuild() override; + WorkResult tryLocalBuild(bool inBuildSlot) override; /** * Start building a derivation. diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc index 67a5f20bb..cacf3618f 100644 --- a/src/libstore/build/substitution-goal.cc +++ b/src/libstore/build/substitution-goal.cc @@ -39,13 +39,13 @@ Goal::Finished PathSubstitutionGoal::done( } -Goal::WorkResult PathSubstitutionGoal::work() +Goal::WorkResult PathSubstitutionGoal::work(bool inBuildSlot) { - return (this->*state)(); + return (this->*state)(inBuildSlot); } -Goal::WorkResult PathSubstitutionGoal::init() +Goal::WorkResult PathSubstitutionGoal::init(bool inBuildSlot) { trace("init"); @@ -61,11 +61,11 @@ Goal::WorkResult PathSubstitutionGoal::init() subs = settings.useSubstitutes ? getDefaultSubstituters() : std::list>(); - return tryNext(); + return tryNext(inBuildSlot); } -Goal::WorkResult PathSubstitutionGoal::tryNext() +Goal::WorkResult PathSubstitutionGoal::tryNext(bool inBuildSlot) { trace("trying next substituter"); @@ -97,23 +97,23 @@ Goal::WorkResult PathSubstitutionGoal::tryNext() if (sub->storeDir == worker.store.storeDir) assert(subPath == storePath); } else if (sub->storeDir != worker.store.storeDir) { - return tryNext(); + return tryNext(inBuildSlot); } try { // FIXME: make async info = sub->queryPathInfo(subPath ? *subPath : storePath); } catch (InvalidPath &) { - return tryNext(); + return tryNext(inBuildSlot); } catch (SubstituterDisabled &) { if (settings.tryFallback) { - return tryNext(); + return tryNext(inBuildSlot); } throw; } catch (Error & e) { if (settings.tryFallback) { logError(e.info()); - return tryNext(); + return tryNext(inBuildSlot); } throw; } @@ -126,7 +126,7 @@ Goal::WorkResult PathSubstitutionGoal::tryNext() } else { printError("asked '%s' for '%s' but got '%s'", sub->getUri(), worker.store.printStorePath(storePath), sub->printStorePath(info->path)); - return tryNext(); + return tryNext(inBuildSlot); } } @@ -147,7 +147,7 @@ Goal::WorkResult PathSubstitutionGoal::tryNext() { warn("ignoring substitute for '%s' from '%s', as it's not signed by any of the keys in 'trusted-public-keys'", worker.store.printStorePath(storePath), sub->getUri()); - return tryNext(); + return tryNext(inBuildSlot); } /* To maintain the closure invariant, we first have to realise the @@ -158,7 +158,7 @@ Goal::WorkResult PathSubstitutionGoal::tryNext() result.goals.insert(worker.makePathSubstitutionGoal(i)); if (result.goals.empty()) {/* to prevent hang (no wake-up event) */ - return referencesValid(); + return referencesValid(inBuildSlot); } else { state = &PathSubstitutionGoal::referencesValid; return result; @@ -166,7 +166,7 @@ Goal::WorkResult PathSubstitutionGoal::tryNext() } -Goal::WorkResult PathSubstitutionGoal::referencesValid() +Goal::WorkResult PathSubstitutionGoal::referencesValid(bool inBuildSlot) { trace("all references realised"); @@ -186,14 +186,11 @@ Goal::WorkResult PathSubstitutionGoal::referencesValid() } -Goal::WorkResult PathSubstitutionGoal::tryToRun() +Goal::WorkResult PathSubstitutionGoal::tryToRun(bool inBuildSlot) { trace("trying to run"); - /* Make sure that we are allowed to start a substitution. Note that even - if maxSubstitutionJobs == 0, we still allow a substituter to run. This - prevents infinite waiting. */ - if (worker.getNrSubstitutions() >= std::max(1U, (unsigned int) settings.maxSubstitutionJobs)) { + if (!inBuildSlot) { return WaitForSlot{}; } @@ -231,7 +228,7 @@ Goal::WorkResult PathSubstitutionGoal::tryToRun() } -Goal::WorkResult PathSubstitutionGoal::finished() +Goal::WorkResult PathSubstitutionGoal::finished(bool inBuildSlot) { trace("substitute finished"); diff --git a/src/libstore/build/substitution-goal.hh b/src/libstore/build/substitution-goal.hh index 0260888ff..e546fc06f 100644 --- a/src/libstore/build/substitution-goal.hh +++ b/src/libstore/build/substitution-goal.hh @@ -66,7 +66,7 @@ struct PathSubstitutionGoal : public Goal std::unique_ptr> maintainExpectedSubstitutions, maintainRunningSubstitutions, maintainExpectedNar, maintainExpectedDownload; - typedef WorkResult (PathSubstitutionGoal::*GoalState)(); + typedef WorkResult (PathSubstitutionGoal::*GoalState)(bool inBuildSlot); GoalState state; /** @@ -94,16 +94,16 @@ public: return "a$" + std::string(storePath.name()) + "$" + worker.store.printStorePath(storePath); } - WorkResult work() override; + WorkResult work(bool inBuildSlot) override; /** * The states. */ - WorkResult init(); - WorkResult tryNext(); - WorkResult referencesValid(); - WorkResult tryToRun(); - WorkResult finished(); + WorkResult init(bool inBuildSlot); + WorkResult tryNext(bool inBuildSlot); + WorkResult referencesValid(bool inBuildSlot); + WorkResult tryToRun(bool inBuildSlot); + WorkResult finished(bool inBuildSlot); /** * Callback used by the worker to write to the log. diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index a27cb0076..411525d94 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -232,18 +232,6 @@ void Worker::wakeUp(GoalPtr goal) } -unsigned Worker::getNrLocalBuilds() -{ - return nrLocalBuilds; -} - - -unsigned Worker::getNrSubstitutions() -{ - return nrSubstitutions; -} - - void Worker::childStarted(GoalPtr goal, const std::set & fds, bool inBuildSlot, bool respectTimeouts) { @@ -307,8 +295,8 @@ void Worker::waitForBuildSlot(GoalPtr goal) { goal->trace("wait for build slot"); bool isSubstitutionGoal = goal->jobCategory() == JobCategory::Substitution; - if ((!isSubstitutionGoal && getNrLocalBuilds() < settings.maxBuildJobs) || - (isSubstitutionGoal && getNrSubstitutions() < settings.maxSubstitutionJobs)) + if ((!isSubstitutionGoal && nrLocalBuilds < settings.maxBuildJobs) || + (isSubstitutionGoal && nrSubstitutions < settings.maxSubstitutionJobs)) wakeUp(goal); /* we can do it right away */ else wantingToBuild.insert(goal); @@ -364,7 +352,12 @@ void Worker::run(const Goals & _topGoals) awake.clear(); for (auto & goal : awake2) { checkInterrupt(); - handleWorkResult(goal, goal->work()); + /* Make sure that we are always allowed to run at least one substitution. + This prevents infinite waiting. */ + const bool inSlot = goal->jobCategory() == JobCategory::Substitution + ? nrSubstitutions < std::max(1U, (unsigned int) settings.maxSubstitutionJobs) + : nrLocalBuilds < settings.maxBuildJobs; + handleWorkResult(goal, goal->work(inSlot)); actDerivations.progress( doneBuilds, expectedBuilds + doneBuilds, runningBuilds, failedBuilds diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh index e21e91385..5f80880a9 100644 --- a/src/libstore/build/worker.hh +++ b/src/libstore/build/worker.hh @@ -229,17 +229,6 @@ public: */ GoalPtr makeGoal(const DerivedPath & req, BuildMode buildMode = bmNormal); - /** - * Return the number of local build processes currently running (but not - * remote builds via the build hook). - */ - unsigned int getNrLocalBuilds(); - - /** - * Return the number of substitution processes currently running. - */ - unsigned int getNrSubstitutions(); - /** * Registers a running child process. `inBuildSlot` means that * the process counts towards the jobs limit.