From 8cffec84859cec8b610a2a22ab0c4d462a9351ff Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 8 Apr 2016 18:16:53 +0200 Subject: [PATCH] Remove failed build caching This feature was implemented for Hydra, but Hydra no longer uses it. --- doc/manual/command-ref/conf-file.xml | 15 ------ doc/manual/command-ref/nix-store.xml | 76 ---------------------------- src/libstore/binary-cache-store.hh | 6 --- src/libstore/build.cc | 41 +-------------- src/libstore/globals.cc | 2 - src/libstore/globals.hh | 3 -- src/libstore/local-store.cc | 66 +++--------------------- src/libstore/local-store.hh | 19 +------ src/libstore/remote-store.cc | 18 +------ src/libstore/remote-store.hh | 4 -- src/libstore/schema.sql | 5 -- src/libstore/store-api.hh | 8 --- src/nix-daemon/nix-daemon.cc | 17 ------- src/nix-store/nix-store.cc | 22 -------- tests/local.mk | 2 +- tests/negative-caching.nix | 21 -------- tests/negative-caching.sh | 22 -------- 17 files changed, 12 insertions(+), 335 deletions(-) delete mode 100644 tests/negative-caching.nix delete mode 100644 tests/negative-caching.sh diff --git a/doc/manual/command-ref/conf-file.xml b/doc/manual/command-ref/conf-file.xml index acddd63e1..f598d359e 100644 --- a/doc/manual/command-ref/conf-file.xml +++ b/doc/manual/command-ref/conf-file.xml @@ -306,21 +306,6 @@ flag, e.g. --option gc-keep-outputs false. - build-cache-failure - - If set to true, Nix will - “cache” build failures, meaning that it will remember (in its - database) that a derivation previously failed. If you then try to - build the derivation again, Nix will immediately fail rather than - perform the build again. Failures in fixed-output derivations - (such as fetchurl calls) are never cached. - The “failed” status of a derivation can be cleared using - nix-store --clear-failed-paths. By default, - failure caching is disabled. - - - - build-keep-log If set to true (the default), diff --git a/doc/manual/command-ref/nix-store.xml b/doc/manual/command-ref/nix-store.xml index 58a331179..340f61210 100644 --- a/doc/manual/command-ref/nix-store.xml +++ b/doc/manual/command-ref/nix-store.xml @@ -1348,82 +1348,6 @@ export _args; _args='-e /nix/store/9krlzvny65gdc8s7kpb6lkx8cd02c25c-default-buil - - -Operation <option>--query-failed-paths</option> - - - Synopsis - - nix-store - - - - -Description - -If build failure caching is enabled through the -build-cache-failure configuration option, the -operation will print out all -store paths that have failed to build. - - - -Example - - -$ nix-store --query-failed-paths -/nix/store/000zi5dcla86l92jn1g997jb06sidm7x-perl-PerlMagick-6.59 -/nix/store/0011iy7sfwbc1qj5a1f6ifjnbcdail8a-haskell-gitit-ghc7.0.4-0.8.1 -/nix/store/001c0yn1hkh86gprvrb46cxnz3pki7q3-gamin-0.1.10 -… - - - - - - - - - -Operation <option>--clear-failed-paths</option> - - - Synopsis - - nix-store - - paths - - - -Description - -If build failure caching is enabled through the -build-cache-failure configuration option, the -operation clears the “failed” -state of the given store paths, allowing them to be built again. This -is useful if the failure was actually transient (e.g. because the disk -was full). - -If a path denotes a derivation, its output paths are cleared. -You can provide the argument * to clear all store -paths. - - - -Example - - -$ nix-store --clear-failed-paths /nix/store/000zi5dcla86l92jn1g997jb06sidm7x-perl-PerlMagick-6.59 -$ nix-store --clear-failed-paths * - - - - - - - Operation <option>--generate-binary-cache-key</option> diff --git a/src/libstore/binary-cache-store.hh b/src/libstore/binary-cache-store.hh index 0020f89ee..9e7b0ad9a 100644 --- a/src/libstore/binary-cache-store.hh +++ b/src/libstore/binary-cache-store.hh @@ -156,12 +156,6 @@ public: void collectGarbage(const GCOptions & options, GCResults & results) override { notImpl(); } - PathSet queryFailedPaths() override - { return {}; } - - void clearFailedPaths(const PathSet & paths) override - { } - void optimiseStore() override { } diff --git a/src/libstore/build.cc b/src/libstore/build.cc index e493ac1aa..c1eea87e3 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -1047,11 +1047,6 @@ void DerivationGoal::haveDerivation() return; } - /* Check whether any output previously failed to build. If so, - don't bother. */ - for (auto & i : invalidOutputs) - if (pathFailed(i)) return; - /* Reject doing a hash build of anything other than a fixed-output derivation. */ if (buildMode == bmHash) { @@ -1322,12 +1317,6 @@ void DerivationGoal::tryToBuild() deletePath(path); } - /* Check again whether any output previously failed to build, - because some other process may have tried and failed before we - acquired the lock. */ - for (auto & i : drv->outputs) - if (pathFailed(i.second.path)) return; - /* Don't do a remote build if the derivation has the attribute `preferLocalBuild' set. Also, check and repair modes are only supported for local builds. */ @@ -1549,17 +1538,6 @@ void DerivationGoal::buildDone() 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). - However, don't do this for fixed-output derivations, - since they're likely to fail for transient reasons - (e.g., fetchurl not being able to access the network). - Hook errors (like communication problems with the - remote machine) shouldn't be cached either. */ - if (settings.cacheFailure && !fixedOutput && !diskFull) - for (auto & i : drv->outputs) - worker.store.registerFailedPath(i.second.path); } done(st, e.msg()); @@ -2993,23 +2971,6 @@ PathSet DerivationGoal::checkPathValidity(bool returnValid, bool checkHash) } -bool DerivationGoal::pathFailed(const Path & path) -{ - if (!settings.cacheFailure) return false; - - if (!worker.store.hasPathFailed(path)) return false; - - printMsg(lvlError, format("builder for ‘%1%’ failed previously (cached)") % path); - - if (settings.printBuildTrace) - printMsg(lvlError, format("@ build-failed %1% - cached") % drvPath); - - done(BuildResult::CachedFailure); - - return true; -} - - Path DerivationGoal::addHashRewrite(const Path & path) { string h1 = string(path, settings.nixStore.size() + 1, 32); @@ -3031,7 +2992,7 @@ void DerivationGoal::done(BuildResult::Status status, const string & msg) amDone(result.success() ? ecSuccess : ecFailed); if (result.status == BuildResult::TimedOut) worker.timedOut = true; - if (result.status == BuildResult::PermanentFailure || result.status == BuildResult::CachedFailure) + if (result.status == BuildResult::PermanentFailure) worker.permanentFailure = true; } diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index e704837e8..e55144a06 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -52,7 +52,6 @@ Settings::Settings() keepLog = true; compressLog = true; maxLogSize = 0; - cacheFailure = false; pollInterval = 5; checkRootReachability = false; gcKeepOutputs = false; @@ -175,7 +174,6 @@ void Settings::update() _get(keepLog, "build-keep-log"); _get(compressLog, "build-compress-log"); _get(maxLogSize, "build-max-log-size"); - _get(cacheFailure, "build-cache-failure"); _get(pollInterval, "build-poll-interval"); _get(checkRootReachability, "gc-check-reachability"); _get(gcKeepOutputs, "gc-keep-outputs"); diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh index 60b11afe6..572fa7188 100644 --- a/src/libstore/globals.hh +++ b/src/libstore/globals.hh @@ -168,9 +168,6 @@ struct Settings { before being killed (0 means no limit). */ unsigned long maxLogSize; - /* Whether to cache build failures. */ - bool cacheFailure; - /* How often (in seconds) to poll for locks. */ unsigned int pollInterval; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 31aee0dd1..c928e7cb6 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -198,6 +198,13 @@ LocalStore::LocalStore() txn.commit(); } + if (curSchema < 9) { + SQLiteTxn txn(state->db); + if (sqlite3_exec(state->db, "drop table FailedPaths", 0, 0, 0) != SQLITE_OK) + throwSQLiteError(state->db, "upgrading database schema"); + txn.commit(); + } + writeFile(schemaPath, (format("%1%") % nixSchemaVersion).str()); lockFile(globalLock, ltRead, true); @@ -327,16 +334,6 @@ void LocalStore::openDB(State & state, bool create) "select path from Refs join ValidPaths on referrer = id where reference = (select id from ValidPaths where path = ?);"); state.stmtInvalidatePath.create(db, "delete from ValidPaths where path = ?;"); - state.stmtRegisterFailedPath.create(db, - "insert or ignore into FailedPaths (path, time) values (?, ?);"); - state.stmtHasPathFailed.create(db, - "select time from FailedPaths where path = ?;"); - state.stmtQueryFailedPaths.create(db, - "select path from FailedPaths;"); - // If the path is a derivation, then clear its outputs. - state.stmtClearFailedPath.create(db, - "delete from FailedPaths where ?1 = '*' or path = ?1 " - "or path in (select d.path from DerivationOutputs d join ValidPaths v on d.drv = v.id where v.path = ?1);"); state.stmtAddDerivationOutput.create(db, "insert or replace into DerivationOutputs (drv, id, path) values (?, ?, ?);"); state.stmtQueryValidDerivers.create(db, @@ -583,55 +580,6 @@ uint64_t LocalStore::addValidPath(State & state, } -void LocalStore::registerFailedPath(const Path & path) -{ - retrySQLite([&]() { - auto state(_state.lock()); - state->stmtRegisterFailedPath.use()(path)(time(0)).step(); - }); -} - - -bool LocalStore::hasPathFailed(const Path & path) -{ - return retrySQLite([&]() { - auto state(_state.lock()); - return state->stmtHasPathFailed.use()(path).next(); - }); -} - - -PathSet LocalStore::queryFailedPaths() -{ - return retrySQLite([&]() { - auto state(_state.lock()); - - auto useQueryFailedPaths(state->stmtQueryFailedPaths.use()); - - PathSet res; - while (useQueryFailedPaths.next()) - res.insert(useQueryFailedPaths.getStr(0)); - - return res; - }); -} - - -void LocalStore::clearFailedPaths(const PathSet & paths) -{ - retrySQLite([&]() { - auto state(_state.lock()); - - SQLiteTxn txn(state->db); - - for (auto & path : paths) - state->stmtClearFailedPath.use()(path).exec(); - - txn.commit(); - }); -} - - Hash parseHashField(const Path & path, const string & s) { string::size_type colon = s.find(':'); diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index dceeb42cc..14ff92c35 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -17,8 +17,8 @@ namespace nix { /* Nix store and database schema version. Version 1 (or 0) was Nix <= 0.7. Version 2 was Nix 0.8 and 0.9. Version 3 is Nix 0.10. Version 4 is Nix 0.11. Version 5 is Nix 0.12-0.16. Version 6 is - Nix 1.0. Version 7 is Nix 1.3. Version 8 is 1.12. */ -const int nixSchemaVersion = 8; + Nix 1.0. Version 7 is Nix 1.3. Version 9 is 1.12. */ +const int nixSchemaVersion = 9; extern string drvsLogDir; @@ -71,10 +71,6 @@ private: SQLiteStmt stmtQueryReferences; SQLiteStmt stmtQueryReferrers; SQLiteStmt stmtInvalidatePath; - SQLiteStmt stmtRegisterFailedPath; - SQLiteStmt stmtHasPathFailed; - SQLiteStmt stmtQueryFailedPaths; - SQLiteStmt stmtClearFailedPath; SQLiteStmt stmtAddDerivationOutput; SQLiteStmt stmtQueryValidDerivers; SQLiteStmt stmtQueryDerivationOutputs; @@ -194,17 +190,6 @@ public: void registerValidPaths(const ValidPathInfos & infos); - /* Register that the build of a derivation with output `path' has - failed. */ - void registerFailedPath(const Path & path); - - /* Query whether `path' previously failed to build. */ - bool hasPathFailed(const Path & path); - - PathSet queryFailedPaths() override; - - void clearFailedPaths(const PathSet & paths) override; - void vacuumDB(); /* Repair the contents of the given path by redownloading it using diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 4d5d689dc..761e83548 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -520,23 +520,6 @@ void RemoteStore::collectGarbage(const GCOptions & options, GCResults & results) } -PathSet RemoteStore::queryFailedPaths() -{ - auto conn(connections->get()); - conn->to << wopQueryFailedPaths; - conn->processStderr(); - return readStorePaths(conn->from); -} - - -void RemoteStore::clearFailedPaths(const PathSet & paths) -{ - auto conn(connections->get()); - conn->to << wopClearFailedPaths << paths; - conn->processStderr(); - readInt(conn->from); -} - void RemoteStore::optimiseStore() { auto conn(connections->get()); @@ -545,6 +528,7 @@ void RemoteStore::optimiseStore() readInt(conn->from); } + bool RemoteStore::verifyStore(bool checkContents, bool repair) { auto conn(connections->get()); diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index cede4d332..45bc41804 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -85,10 +85,6 @@ public: void collectGarbage(const GCOptions & options, GCResults & results) override; - PathSet queryFailedPaths() override; - - void clearFailedPaths(const PathSet & paths) override; - void optimiseStore() override; bool verifyStore(bool checkContents, bool repair) override; diff --git a/src/libstore/schema.sql b/src/libstore/schema.sql index 39c74c65a..91878af15 100644 --- a/src/libstore/schema.sql +++ b/src/libstore/schema.sql @@ -39,8 +39,3 @@ create table if not exists DerivationOutputs ( ); create index if not exists IndexDerivationOutputs on DerivationOutputs(path); - -create table if not exists FailedPaths ( - path text primary key not null, - time integer not null -); diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 798054d16..ae5631ba0 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -148,7 +148,6 @@ struct BuildResult InputRejected, OutputRejected, TransientFailure, // possibly transient - CachedFailure, TimedOut, MiscFailure, DependencyFailed, @@ -325,13 +324,6 @@ public: /* Perform a garbage collection. */ virtual void collectGarbage(const GCOptions & options, GCResults & results) = 0; - /* Return the set of paths that have failed to build.*/ - virtual PathSet queryFailedPaths() = 0; - - /* Clear the "failed" status of the given paths. The special - value `*' causes all failed paths to be cleared. */ - virtual void clearFailedPaths(const PathSet & paths) = 0; - /* Return a string representing information about the path that can be loaded into the database using `nix-store --load-db' or `nix-store --register-validity'. */ diff --git a/src/nix-daemon/nix-daemon.cc b/src/nix-daemon/nix-daemon.cc index 439cd3dc0..c3cdb8395 100644 --- a/src/nix-daemon/nix-daemon.cc +++ b/src/nix-daemon/nix-daemon.cc @@ -493,23 +493,6 @@ static void performOp(ref store, bool trusted, unsigned int clientVe break; } - case wopQueryFailedPaths: { - startWork(); - PathSet paths = store->queryFailedPaths(); - stopWork(); - to << paths; - break; - } - - case wopClearFailedPaths: { - PathSet paths = readStrings(from); - startWork(); - store->clearFailedPaths(paths); - stopWork(); - to << 1; - break; - } - case wopQueryPathInfo: { Path path = readStorePath(from); startWork(); diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 1a2723041..179015b52 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -821,24 +821,6 @@ static void opOptimise(Strings opFlags, Strings opArgs) store->optimiseStore(); } -static void opQueryFailedPaths(Strings opFlags, Strings opArgs) -{ - if (!opArgs.empty() || !opFlags.empty()) - throw UsageError("no arguments expected"); - PathSet failed = store->queryFailedPaths(); - for (auto & i : failed) - cout << format("%1%\n") % i; -} - - -static void opClearFailedPaths(Strings opFlags, Strings opArgs) -{ - if (!opFlags.empty()) - throw UsageError("no flags expected"); - store->clearFailedPaths(PathSet(opArgs.begin(), opArgs.end())); -} - - /* Serve the nix store in a way usable by a restricted ssh user. */ static void opServe(Strings opFlags, Strings opArgs) { @@ -1102,10 +1084,6 @@ int main(int argc, char * * argv) op = opRepairPath; else if (*arg == "--optimise" || *arg == "--optimize") op = opOptimise; - else if (*arg == "--query-failed-paths") - op = opQueryFailedPaths; - else if (*arg == "--clear-failed-paths") - op = opClearFailedPaths; else if (*arg == "--serve") op = opServe; else if (*arg == "--generate-binary-cache-key") diff --git a/tests/local.mk b/tests/local.mk index 03f53b44c..46f339de6 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -7,7 +7,7 @@ nix_tests = \ fallback.sh nix-push.sh gc.sh gc-concurrent.sh nix-pull.sh \ referrers.sh user-envs.sh logging.sh nix-build.sh misc.sh fixed.sh \ gc-runtime.sh install-package.sh check-refs.sh filter-source.sh \ - remote-store.sh export.sh export-graph.sh negative-caching.sh \ + remote-store.sh export.sh export-graph.sh \ binary-patching.sh timeout.sh secure-drv-outputs.sh nix-channel.sh \ multiple-outputs.sh import-derivation.sh fetchurl.sh optimise-store.sh \ binary-cache.sh nix-profile.sh repair.sh dump-db.sh case-hack.sh \ diff --git a/tests/negative-caching.nix b/tests/negative-caching.nix deleted file mode 100644 index 10df67a74..000000000 --- a/tests/negative-caching.nix +++ /dev/null @@ -1,21 +0,0 @@ -with import ./config.nix; - -rec { - - fail = mkDerivation { - name = "fail"; - builder = builtins.toFile "builder.sh" "echo FAIL; exit 1"; - }; - - succeed = mkDerivation { - name = "succeed"; - builder = builtins.toFile "builder.sh" "echo SUCCEED; mkdir $out"; - }; - - depOnFail = mkDerivation { - name = "dep-on-fail"; - builder = builtins.toFile "builder.sh" "echo URGH; mkdir $out"; - inputs = [fail succeed]; - }; - -} diff --git a/tests/negative-caching.sh b/tests/negative-caching.sh deleted file mode 100644 index 4217bc38e..000000000 --- a/tests/negative-caching.sh +++ /dev/null @@ -1,22 +0,0 @@ -source common.sh - -clearStore - -set +e - -opts="--option build-cache-failure true --print-build-trace" - -# This build should fail, and the failure should be cached. -log=$(nix-build $opts negative-caching.nix -A fail --no-out-link 2>&1) && fail "should fail" -echo "$log" | grep -q "@ build-failed" || fail "no build-failed trace" - -# Do it again. The build shouldn't be tried again. -log=$(nix-build $opts negative-caching.nix -A fail --no-out-link 2>&1) && fail "should fail" -echo "$log" | grep -q "FAIL" && fail "failed build not cached" -echo "$log" | grep -q "@ build-failed .* cached" || fail "trace doesn't say cached" - -# Check that --keep-going works properly with cached failures. -log=$(nix-build $opts --keep-going negative-caching.nix -A depOnFail --no-out-link 2>&1) && fail "should fail" -echo "$log" | grep -q "FAIL" && fail "failed build not cached (2)" -echo "$log" | grep -q "@ build-failed .* cached" || fail "trace doesn't say cached (2)" -echo "$log" | grep -q "@ build-succeeded .*-succeed" || fail "didn't keep going"