From eda2f36c2ac847e02e871c327e7633693d92cd8d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 20 Jul 2015 03:15:45 +0200 Subject: [PATCH] Provide more detailed info about build status to hydra-queue-runner In particular, hydra-queue-runner can now distinguish between remote build / substitution / already-valid. For instance, if a path already existed on the remote side, we don't want to store a log file. --- src/libstore/build.cc | 104 ++++++++++++++++++++++---------------- src/libstore/store-api.hh | 18 +++++-- 2 files changed, 75 insertions(+), 47 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 8f1d5b2c8..61677d1eb 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -94,6 +94,7 @@ struct HookInstance; /* A pointer to a goal. */ class Goal; +class DerivationGoal; typedef std::shared_ptr GoalPtr; typedef std::weak_ptr WeakGoalPtr; @@ -184,10 +185,10 @@ public: return exitCode; } - /* Cancel the goal. It should wake up its waiters, get rid of any - running child processes that are being monitored by the worker - (important!), etc. */ - virtual void cancel(bool timeout) = 0; + /* Callback in case of a timeout. It should wake up its waiters, + get rid of any running child processes that are being monitored + by the worker (important!), etc. */ + virtual void timedOut() = 0; virtual string key() = 0; @@ -275,7 +276,8 @@ public: /* Make a goal (with caching). */ GoalPtr makeDerivationGoal(const Path & drvPath, const StringSet & wantedOutputs, BuildMode buildMode = bmNormal); - GoalPtr makeBasicDerivationGoal(const Path & drvPath, const BasicDerivation & drv, BuildMode buildMode = bmNormal); + std::shared_ptr makeBasicDerivationGoal(const Path & drvPath, + const BasicDerivation & drv, BuildMode buildMode = bmNormal); GoalPtr makeSubstitutionGoal(const Path & storePath, bool repair = false); /* Remove a dead goal. */ @@ -792,6 +794,8 @@ private: outputs to allow hard links between outputs. */ InodesSeen inodesSeen; + BuildResult result; + public: DerivationGoal(const Path & drvPath, const StringSet & wantedOutputs, Worker & worker, BuildMode buildMode = bmNormal); @@ -799,7 +803,7 @@ public: Worker & worker, BuildMode buildMode = bmNormal); ~DerivationGoal(); - void cancel(bool timeout); + void timedOut() override; string key() { @@ -820,6 +824,8 @@ public: /* Add wanted outputs to an already existing derivation goal. */ void addWantedOutputs(const StringSet & outputs); + BuildResult getResult() { return result; } + private: /* The states. */ void getDerivation(); @@ -871,6 +877,8 @@ private: Path addHashRewrite(const Path & path); void repairClosure(); + + void done(BuildResult::Status status, const string & msg = ""); }; @@ -937,12 +945,12 @@ void DerivationGoal::killChild() } -void DerivationGoal::cancel(bool timeout) +void DerivationGoal::timedOut() { - if (settings.printBuildTrace && timeout) + if (settings.printBuildTrace) printMsg(lvlError, format("@ build-failed %1% - timeout") % drvPath); killChild(); - amDone(ecFailed); + done(BuildResult::TimedOut); } @@ -993,7 +1001,7 @@ void DerivationGoal::loadDerivation() if (nrFailed != 0) { printMsg(lvlError, format("cannot build missing derivation ‘%1%’") % drvPath); - amDone(ecFailed); + done(BuildResult::MiscFailure); return; } @@ -1023,7 +1031,7 @@ void DerivationGoal::haveDerivation() /* If they are all valid, then we're done. */ if (invalidOutputs.size() == 0 && buildMode == bmNormal) { - amDone(ecSuccess); + done(BuildResult::AlreadyValid); return; } @@ -1068,7 +1076,7 @@ void DerivationGoal::outputsSubstituted() unsigned int nrInvalid = checkPathValidity(false, buildMode == bmRepair).size(); if (buildMode == bmNormal && nrInvalid == 0) { - amDone(ecSuccess); + done(BuildResult::Substituted); return; } if (buildMode == bmRepair && nrInvalid == 0) { @@ -1147,7 +1155,7 @@ void DerivationGoal::repairClosure() } if (waitees.empty()) { - amDone(ecSuccess); + done(BuildResult::AlreadyValid); return; } @@ -1160,7 +1168,7 @@ void DerivationGoal::closureRepaired() trace("closure repaired"); if (nrFailed > 0) throw Error(format("some paths in the output closure of derivation ‘%1%’ could not be repaired") % drvPath); - amDone(ecSuccess); + done(BuildResult::AlreadyValid); } @@ -1174,7 +1182,7 @@ void DerivationGoal::inputsRealised() printMsg(lvlError, format("cannot build derivation ‘%1%’: %2% dependencies couldn't be built") % drvPath % nrFailed); - amDone(ecFailed); + done(BuildResult::DependencyFailed); return; } @@ -1300,7 +1308,7 @@ void DerivationGoal::tryToBuild() if (buildMode != bmCheck && validPaths.size() == drv->outputs.size()) { debug(format("skipping build of derivation ‘%1%’, someone beat us to it") % drvPath); outputLocks.setDeletion(true); - amDone(ecSuccess); + done(BuildResult::AlreadyValid); return; } @@ -1372,7 +1380,7 @@ void DerivationGoal::tryToBuild() printMsg(lvlError, format("@ build-failed %1% - %2% %3%") % drvPath % 0 % e.msg()); worker.permanentFailure = true; - amDone(ecFailed); + done(BuildResult::InputRejected, e.msg()); return; } @@ -1484,7 +1492,7 @@ void DerivationGoal::buildDone() registerOutputs(); if (buildMode == bmCheck) { - amDone(ecSuccess); + done(BuildResult::Built); return; } @@ -1510,10 +1518,12 @@ void DerivationGoal::buildDone() outputLocks.unlock(); buildUser.release(); + BuildResult::Status st = BuildResult::MiscFailure; + if (hook && WIFEXITED(status) && WEXITSTATUS(status) == 101) { if (settings.printBuildTrace) printMsg(lvlError, format("@ build-failed %1% - timeout") % drvPath); - worker.timedOut = true; + st = BuildResult::TimedOut; } else if (hook && (!WIFEXITED(status) || WEXITSTATUS(status) != 100)) { @@ -1526,7 +1536,11 @@ void DerivationGoal::buildDone() if (settings.printBuildTrace) printMsg(lvlError, format("@ build-failed %1% - %2% %3%") % drvPath % 1 % e.msg()); - worker.permanentFailure = !fixedOutput && !diskFull; + + st = + statusOk(status) ? BuildResult::OutputRejected : + fixedOutput || diskFull ? BuildResult::TransientFailure : + BuildResult::PermanentFailure; /* Register the outputs of this build as "failed" so we won't try to build them again (negative caching). @@ -1540,7 +1554,7 @@ void DerivationGoal::buildDone() worker.store.registerFailedPath(i.second.path); } - amDone(ecFailed); + done(st, e.msg()); return; } @@ -1550,7 +1564,7 @@ void DerivationGoal::buildDone() if (settings.printBuildTrace) printMsg(lvlError, format("@ build-succeeded %1% -") % drvPath); - amDone(ecSuccess); + done(BuildResult::Built); } @@ -2810,7 +2824,7 @@ void DerivationGoal::handleChildOutput(int fd, const string & data) printMsg(lvlError, format("%1% killed after writing more than %2% bytes of log output") % getName() % settings.maxLogSize); - cancel(true); // not really a timeout, but close enough + timedOut(); // not really a timeout, but close enough return; } if (verbosity >= settings.buildVerbosity) @@ -2859,8 +2873,7 @@ bool DerivationGoal::pathFailed(const Path & path) if (settings.printBuildTrace) printMsg(lvlError, format("@ build-failed %1% - cached") % drvPath); - worker.permanentFailure = true; - amDone(ecFailed); + done(BuildResult::CachedFailure); return true; } @@ -2880,6 +2893,18 @@ Path DerivationGoal::addHashRewrite(const Path & path) } +void DerivationGoal::done(BuildResult::Status status, const string & msg) +{ + result.status = status; + result.errorMsg = msg; + amDone(result.success() ? ecSuccess : ecFailed); + if (result.status == BuildResult::TimedOut) + worker.timedOut = true; + if (result.status == BuildResult::PermanentFailure || result.status == BuildResult::CachedFailure) + worker.permanentFailure = true; +} + + ////////////////////////////////////////////////////////////////////// @@ -2929,7 +2954,7 @@ public: SubstitutionGoal(const Path & storePath, Worker & worker, bool repair = false); ~SubstitutionGoal(); - void cancel(bool timeout); + void timedOut(); string key() { @@ -2974,9 +2999,9 @@ SubstitutionGoal::~SubstitutionGoal() } -void SubstitutionGoal::cancel(bool timeout) +void SubstitutionGoal::timedOut() { - if (settings.printBuildTrace && timeout) + if (settings.printBuildTrace) printMsg(lvlError, format("@ substituter-failed %1% timeout") % storePath); if (pid != -1) { pid_t savedPid = pid; @@ -3297,7 +3322,8 @@ Worker::~Worker() } -GoalPtr Worker::makeDerivationGoal(const Path & path, const StringSet & wantedOutputs, BuildMode buildMode) +GoalPtr Worker::makeDerivationGoal(const Path & path, + const StringSet & wantedOutputs, BuildMode buildMode) { GoalPtr goal = derivationGoals[path].lock(); if (!goal) { @@ -3310,7 +3336,8 @@ GoalPtr Worker::makeDerivationGoal(const Path & path, const StringSet & wantedOu } -GoalPtr Worker::makeBasicDerivationGoal(const Path & drvPath, const BasicDerivation & drv, BuildMode buildMode) +std::shared_ptr Worker::makeBasicDerivationGoal(const Path & drvPath, + const BasicDerivation & drv, BuildMode buildMode) { auto goal = std::make_shared(drvPath, drv, *this, buildMode); wakeUp(goal); @@ -3562,7 +3589,7 @@ void Worker::waitForInput() /* Since goals may be canceled from inside the loop below (causing them go be erased from the `children' map), we have to be careful that we don't keep iterators alive across calls to - cancel(). */ + timedOut(). */ set pids; for (auto & i : children) pids.insert(i.first); @@ -3604,8 +3631,7 @@ void Worker::waitForInput() printMsg(lvlError, format("%1% timed out after %2% seconds of silence") % goal->getName() % settings.maxSilentTime); - goal->cancel(true); - timedOut = true; + goal->timedOut(); } else if (goal->getExitCode() == Goal::ecBusy && @@ -3616,8 +3642,7 @@ void Worker::waitForInput() printMsg(lvlError, format("%1% timed out after %2% seconds") % goal->getName() % settings.buildTimeout); - goal->cancel(true); - timedOut = true; + goal->timedOut(); } } @@ -3683,14 +3708,7 @@ BuildResult LocalStore::buildDerivation(const Path & drvPath, const BasicDerivat try { worker.run(Goals{goal}); - if (goal->getExitCode() == Goal::ecSuccess) - result.status = BuildResult::Success; - else if (worker.permanentFailure) - result.status = BuildResult::PermanentFailure; - else if (worker.timedOut) - result.status = BuildResult::TimedOut; - else - result.status = BuildResult::MiscFailure; + result = goal->getResult(); } catch (Error & e) { result.status = BuildResult::MiscFailure; result.errorMsg = e.msg(); diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index afc7376fe..d04a040bb 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -103,13 +103,23 @@ enum BuildMode { bmNormal, bmRepair, bmCheck }; struct BuildResult { enum Status { - Success = 0, - PermanentFailure = 1, - TimedOut = 2, - MiscFailure = 3 + Built = 0, + Substituted, + AlreadyValid, + PermanentFailure, + InputRejected, + OutputRejected, + TransientFailure, // possibly transient + CachedFailure, + TimedOut, + MiscFailure, + DependencyFailed } status = MiscFailure; std::string errorMsg; //time_t startTime = 0, stopTime = 0; + bool success() { + return status == Built || status == Substituted || status == AlreadyValid; + } };