diff --git a/doc/manual/rl-next/split-locks.md b/doc/manual/rl-next/split-locks.md new file mode 100644 index 000000000000..bc9261812e13 --- /dev/null +++ b/doc/manual/rl-next/split-locks.md @@ -0,0 +1,12 @@ +--- +synopsis: Split locks +issues: 10740 2589 2029 +prs: 15856 +--- + +New remote only locks are used when [`avoid-local`]{#conf-avoid-local} is true. +To further aid recursive setups, a new builder parameter is added which when +true, disable build hooks (remote builds) on the builder. + +Build hooks will now need to hold the local lock when copying outputs from a +remote builder if [`avoid-local`]{#conf-avoid-local} is true. diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc index 248ef7507c4e..ccef1d715ed4 100644 --- a/src/libstore/build/derivation-building-goal.cc +++ b/src/libstore/build/derivation-building-goal.cc @@ -430,8 +430,14 @@ Goal::Co DerivationBuildingGoal::tryToBuild(StorePathSet inputPaths) return LocalBuildCapability{*localStoreP, ext}; }(); - auto acquireResources = [&](bool & done, PathLocks & outputLocks) -> Goal::Co { - trace("trying to build"); + auto acquireResources = [&](bool & done, bool remote, PathLocks & outputLocks) -> Goal::Co { + // Enable split locks if build hooks are used for all applicable builds + if (!worker.settings.avoidLocal) + remote = false; + if (remote) + trace("trying to build remotely"); + else + trace("trying to build locally"); /** * Output paths to acquire locks on, if known a priori. @@ -447,17 +453,20 @@ Goal::Co DerivationBuildingGoal::tryToBuild(StorePathSet inputPaths) if (auto * localStore = dynamic_cast(&worker.store)) { /* If we aren't a local store, we might need to use the local store as a build remote, but that would cause a deadlock. */ - /* FIXME: Make it so we can use ourselves as a build remote even if we - are the local store (separate locking for building vs scheduling? */ /* FIXME: find some way to lock for scheduling for the other stores so a forking daemon with --store still won't farm out redundant builds. */ for (auto & i : drv->outputsAndOptPaths(worker.store)) { - if (i.second.second) - lockFiles.insert(localStore->toRealPath(*i.second.second)); - else { + if (i.second.second) { + auto lockPath = localStore->toRealPath(*i.second.second); + if (remote) + lockPath += "-remote"; + lockFiles.insert(std::move(lockPath)); + } else { auto lockPath = localStore->toRealPath(drvPath); lockPath += "." + i.first; + if (remote) + lockPath += "-remote"; lockFiles.insert(std::move(lockPath)); } } @@ -513,7 +522,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild(StorePathSet inputPaths) auto tryHookLoop = [&](bool & valid) -> Goal::Co { { PathLocks outputLocks; - co_await acquireResources(valid, outputLocks); + co_await acquireResources(valid, true, outputLocks); if (valid) co_return doneSuccess(BuildResult::Success::AlreadyValid, checkPathValidity(initialOutputs).second); @@ -546,7 +555,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild(StorePathSet inputPaths) while (true) { co_await waitForAWhile(); - co_await acquireResources(valid, outputLocks); + co_await acquireResources(valid, true, outputLocks); if (valid) break; @@ -580,7 +589,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild(StorePathSet inputPaths) auto tryBuildLocally = [&](bool & valid) -> Goal::Co { if (auto * cap = std::get_if(&localBuildResult)) { PathLocks outputLocks; - co_await acquireResources(valid, outputLocks); + co_await acquireResources(valid, false, outputLocks); if (valid) co_return doneSuccess(BuildResult::Success::AlreadyValid, checkPathValidity(initialOutputs).second); @@ -1198,9 +1207,11 @@ HookReply DerivationBuildingGoal::tryBuildHook(const DerivationOptionssink << "try" << (worker.getNrLocalBuilds() < worker.settings.maxBuildJobs ? 1 : 0) - << drv->platform << worker.store.printStorePath(drvPath) + worker.hook->sink << "try" << (canBuildLocally ? 1 : 0) << drv->platform << worker.store.printStorePath(drvPath) << drvOptions.getRequiredSystemFeatures(*drv); worker.hook->sink.flush(); diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index fc4c8a5bb507..f2f43fafc0c8 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -75,7 +75,8 @@ Goal::Co DerivationGoal::haveDerivation(bool storeDerivation) auto checkResult = checkPathValidity(); /* If they are all valid, then we're done. */ - if (checkResult && checkResult->second == PathStatus::Valid && buildMode == bmNormal) { + if (checkResult && checkResult->second == PathStatus::Valid + && (buildMode == bmNormal || buildMode == bmLocal)) { co_return doneSuccess(BuildResult::Success::AlreadyValid, checkResult->first); } @@ -134,7 +135,7 @@ Goal::Co DerivationGoal::haveDerivation(bool storeDerivation) bool allValid = checkResult && checkResult->second == PathStatus::Valid; - if (buildMode == bmNormal && allValid) { + if ((buildMode == bmNormal || buildMode == bmLocal) && allValid) { co_return doneSuccess(BuildResult::Success::Substituted, checkResult->first); } if (buildMode == bmRepair && allValid) { diff --git a/src/libstore/include/nix/store/machines.hh b/src/libstore/include/nix/store/machines.hh index a3e9353c80fa..9f446e826c21 100644 --- a/src/libstore/include/nix/store/machines.hh +++ b/src/libstore/include/nix/store/machines.hh @@ -23,6 +23,7 @@ struct Machine const StringSet supportedFeatures; const StringSet mandatoryFeatures; const std::string sshPublicHostKey; + const bool forceLocal; bool enabled = true; /** @@ -50,7 +51,8 @@ struct Machine decltype(speedFactor) speedFactor, decltype(supportedFeatures) supportedFeatures, decltype(mandatoryFeatures) mandatoryFeatures, - decltype(sshPublicHostKey) sshPublicHostKey); + decltype(sshPublicHostKey) sshPublicHostKey, + decltype(forceLocal) forceLocal); /** * Elaborate `storeUri` into a complete store reference, diff --git a/src/libstore/include/nix/store/store-api.hh b/src/libstore/include/nix/store/store-api.hh index b6114cce65e7..9ed166840af1 100644 --- a/src/libstore/include/nix/store/store-api.hh +++ b/src/libstore/include/nix/store/store-api.hh @@ -50,7 +50,7 @@ enum CheckSigsFlag : bool { NoCheckSigs = false, CheckSigs = true }; enum SubstituteFlag : bool { NoSubstitute = false, Substitute = true }; -enum BuildMode : uint8_t { bmNormal, bmRepair, bmCheck }; +enum BuildMode : uint8_t { bmNormal, bmRepair, bmCheck, bmLocal }; enum TrustedFlag : bool { NotTrusted = false, Trusted = true }; diff --git a/src/libstore/include/nix/store/worker-settings.hh b/src/libstore/include/nix/store/worker-settings.hh index 13fa0261d8d0..997b56e176e3 100644 --- a/src/libstore/include/nix/store/worker-settings.hh +++ b/src/libstore/include/nix/store/worker-settings.hh @@ -198,6 +198,8 @@ public: The value for this field can be obtained via `base64 -w0`. + 9. A boolean which if true, forces the build to not use builders on the remote store + > **Example** > > Multiple builders specified on the command line: @@ -270,6 +272,16 @@ public: This can drastically reduce build times if the network connection between the local machine and the remote build host is slow. )"}; + Setting avoidLocal{ + this, + false, + "avoid-local", + R"( + If set to `true`, Nix will not build derivations locally which can be built by the [`builders`]{#conf-builders}. This does not include derivations with preferLocalBuild set for which [`max-jobs`]{#conf-max-jobs} still applies. + + This enables remote only locks which are not used by local builds + )"}; + Setting useSubstitutes{ this, true, diff --git a/src/libstore/machines.cc b/src/libstore/machines.cc index 17864ca5ace6..e3ee41d0738c 100644 --- a/src/libstore/machines.cc +++ b/src/libstore/machines.cc @@ -14,7 +14,8 @@ Machine::Machine( decltype(speedFactor) speedFactor, decltype(supportedFeatures) supportedFeatures, decltype(mandatoryFeatures) mandatoryFeatures, - decltype(sshPublicHostKey) sshPublicHostKey) + decltype(sshPublicHostKey) sshPublicHostKey, + decltype(forceLocal) forceLocal) : storeUri( StoreReference::parse( // Backwards compatibility: if the URI is schemeless, is not a path, @@ -32,6 +33,7 @@ Machine::Machine( , supportedFeatures(supportedFeatures) , mandatoryFeatures(mandatoryFeatures) , sshPublicHostKey(sshPublicHostKey) + , forceLocal(forceLocal) { if (speedFactor < 0.0) throw UsageError("speed factor must be >= 0"); @@ -188,7 +190,9 @@ static Machine parseBuilderLine(const StringSet & defaultSystems, const std::str // `mandatoryFeatures` isSet(6) ? tokenizeString(tokens[6], ",") : StringSet{}, // `sshPublicHostKey` - isSet(7) ? ensureBase64(7) : ""}; + isSet(7) ? ensureBase64(7) : "", + // `forceLocal` + isSet(8) ? (tokens[8] == "true") : false}; } static Machines parseBuilderLines(const StringSet & defaultSystems, const std::vector & builders) diff --git a/src/libstore/unix/build/chroot-derivation-builder.cc b/src/libstore/unix/build/chroot-derivation-builder.cc index 157d9e319173..a384fc81a6d3 100644 --- a/src/libstore/unix/build/chroot-derivation-builder.cc +++ b/src/libstore/unix/build/chroot-derivation-builder.cc @@ -114,7 +114,7 @@ struct ChrootDerivationBuilder : virtual DerivationBuilderImpl /* Move paths out of the chroot for easier debugging of build failures. */ - if (!force && buildMode == bmNormal) + if (!force && (buildMode == bmNormal || buildMode == bmLocal)) for (auto & [_, status] : initialOutputs) { if (!status.known) continue; diff --git a/src/libstore/worker-protocol.cc b/src/libstore/worker-protocol.cc index 37aa4e31bec2..32aaeb1bb0a7 100644 --- a/src/libstore/worker-protocol.cc +++ b/src/libstore/worker-protocol.cc @@ -67,6 +67,8 @@ BuildMode WorkerProto::Serialise::read(const StoreDirConfig & store, return bmRepair; case 2: return bmCheck; + case 3: + return bmLocal; default: throw Error("Invalid build mode"); } @@ -85,6 +87,9 @@ void WorkerProto::Serialise::write( case bmCheck: conn.to << uint8_t{2}; break; + case bmLocal: + conn.to << uint8_t{3}; + break; default: assert(false); }; diff --git a/src/nix/build-remote/build-remote.cc b/src/nix/build-remote/build-remote.cc index e5ebf948d7e2..d5589aa1c18e 100644 --- a/src/nix/build-remote/build-remote.cc +++ b/src/nix/build-remote/build-remote.cc @@ -110,6 +110,10 @@ static int main_build_remote(int argc, char ** argv) std::shared_ptr sshStore; AutoCloseFD bestSlotLock; + // Whether to force local building on the target store bypassing build + // hooks + bool forceLocal = false; + auto machines = Machine::parseConfig({settings.thisSystem}, settings.getWorkerSettings().builders); debug("got %d remote builders", machines.size()); @@ -252,6 +256,8 @@ static int main_build_remote(int argc, char ** argv) sshStore = bestMachine->openStore(); sshStore->connect(); + + forceLocal = bestMachine->forceLocal; } catch (std::exception & e) { auto msg = chomp(drainFD(5, {.block = false})); printError("cannot build on '%s': %s%s", storeUri, e.what(), msg.empty() ? "" : ": " + msg); @@ -321,6 +327,8 @@ static int main_build_remote(int argc, char ** argv) !trusted || *trusted; }); + auto buildMode = forceLocal ? bmLocal : bmNormal; + // See the very large comment in `case WorkerProto::Op::BuildDerivation:` in // `src/libstore/daemon.cc` that explains the trust model here. // @@ -338,7 +346,7 @@ static int main_build_remote(int argc, char ** argv) // output ids, which break CA derivations if (!drv.inputDrvs.map.empty()) drv.inputSrcs = store->parseStorePathSet(inputs); - optResult = sshStore->buildDerivation(*drvPath, static_cast(drv)); + optResult = sshStore->buildDerivation(*drvPath, static_cast(drv), buildMode); auto & result = *optResult; if (auto * failureP = result.tryGetFailure()) { if (settings.keepFailed) { @@ -353,10 +361,12 @@ static int main_build_remote(int argc, char ** argv) } } else { copyClosure(*store, *sshStore, StorePathSet{*drvPath}, NoRepair, NoCheckSigs, substitute); - auto res = sshStore->buildPathsWithResults({DerivedPath::Built{ - .drvPath = makeConstantStorePathRef(*drvPath), - .outputs = OutputsSpec::All{}, - }}); + auto res = sshStore->buildPathsWithResults( + {DerivedPath::Built{ + .drvPath = makeConstantStorePathRef(*drvPath), + .outputs = OutputsSpec::All{}, + }}, + buildMode); // One path to build should produce exactly one build result assert(res.size() == 1); optResult = std::move(res[0]); @@ -392,9 +402,10 @@ static int main_build_remote(int argc, char ** argv) if (!missingPaths.empty()) { Activity act(*logger, lvlTalkative, actUnknown, fmt("copying outputs from '%s'", storeUri)); - if (auto localStore = store.dynamic_pointer_cast()) - for (auto & path : missingPaths) - localStore->locksHeld.insert(store->printStorePath(path)); /* FIXME: ugly */ + if (!settings.getWorkerSettings().avoidLocal) + if (auto localStore = store.dynamic_pointer_cast()) + for (auto & path : missingPaths) + localStore->locksHeld.insert(store->printStorePath(path)); /* FIXME: ugly */ copyPaths(*sshStore, *store, missingPaths, NoRepair, NoCheckSigs, NoSubstitute); } // XXX: Should be done as part of `copyPaths`