Skip to content

Commit aeba5ab

Browse files
committed
Use dedicated lock files for builds which will use build hooks for
building This allows the build hook to use the local store. In order for the split locks to be used, derivations must either only use the build hook (avoid-local) or only be built locally (forceLocal/bmLocal).
1 parent ff05a94 commit aeba5ab

10 files changed

Lines changed: 68 additions & 20 deletions

File tree

doc/manual/rl-next/split-locks.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
synopsis: Split locks
3+
issues: 10740 2589 2029
4+
---
5+
6+
New remote only locks are used when [`avoid-local`]{#conf-avoid-local} is true.
7+
To further aid recursive setups, a new builder parameter is added which when
8+
true, disable build hooks (remote builds) on the builder.

src/libstore/build/derivation-building-goal.cc

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -430,8 +430,14 @@ Goal::Co DerivationBuildingGoal::tryToBuild(StorePathSet inputPaths)
430430
return LocalBuildCapability{*localStoreP, ext};
431431
}();
432432

433-
auto acquireResources = [&](bool & done, PathLocks & outputLocks) -> Goal::Co {
434-
trace("trying to build");
433+
auto acquireResources = [&](bool & done, bool remote, PathLocks & outputLocks) -> Goal::Co {
434+
// Enable split locks if build hooks are used for all applicable builds
435+
if (!worker.settings.avoidLocal)
436+
remote = false;
437+
if (remote)
438+
trace("trying to build remotely");
439+
else
440+
trace("trying to build locally");
435441

436442
/**
437443
* Output paths to acquire locks on, if known a priori.
@@ -447,17 +453,20 @@ Goal::Co DerivationBuildingGoal::tryToBuild(StorePathSet inputPaths)
447453
if (auto * localStore = dynamic_cast<LocalStore *>(&worker.store)) {
448454
/* If we aren't a local store, we might need to use the local store as
449455
a build remote, but that would cause a deadlock. */
450-
/* FIXME: Make it so we can use ourselves as a build remote even if we
451-
are the local store (separate locking for building vs scheduling? */
452456
/* FIXME: find some way to lock for scheduling for the other stores so
453457
a forking daemon with --store still won't farm out redundant builds.
454458
*/
455459
for (auto & i : drv->outputsAndOptPaths(worker.store)) {
456-
if (i.second.second)
457-
lockFiles.insert(localStore->toRealPath(*i.second.second));
458-
else {
460+
if (i.second.second) {
461+
auto lockPath = localStore->toRealPath(*i.second.second);
462+
if (remote)
463+
lockPath += "-remote";
464+
lockFiles.insert(std::move(lockPath));
465+
} else {
459466
auto lockPath = localStore->toRealPath(drvPath);
460467
lockPath += "." + i.first;
468+
if (remote)
469+
lockPath += "-remote";
461470
lockFiles.insert(std::move(lockPath));
462471
}
463472
}
@@ -513,7 +522,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild(StorePathSet inputPaths)
513522
auto tryHookLoop = [&](bool & valid) -> Goal::Co {
514523
{
515524
PathLocks outputLocks;
516-
co_await acquireResources(valid, outputLocks);
525+
co_await acquireResources(valid, true, outputLocks);
517526
if (valid)
518527
co_return doneSuccess(BuildResult::Success::AlreadyValid, checkPathValidity(initialOutputs).second);
519528

@@ -546,7 +555,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild(StorePathSet inputPaths)
546555

547556
while (true) {
548557
co_await waitForAWhile();
549-
co_await acquireResources(valid, outputLocks);
558+
co_await acquireResources(valid, true, outputLocks);
550559
if (valid)
551560
break;
552561

@@ -580,7 +589,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild(StorePathSet inputPaths)
580589
auto tryBuildLocally = [&](bool & valid) -> Goal::Co {
581590
if (auto * cap = std::get_if<LocalBuildCapability>(&localBuildResult)) {
582591
PathLocks outputLocks;
583-
co_await acquireResources(valid, outputLocks);
592+
co_await acquireResources(valid, false, outputLocks);
584593
if (valid)
585594
co_return doneSuccess(BuildResult::Success::AlreadyValid, checkPathValidity(initialOutputs).second);
586595

@@ -1198,8 +1207,10 @@ HookReply DerivationBuildingGoal::tryBuildHook(const DerivationOptions<StorePath
11981207

11991208
try {
12001209

1210+
auto canBuildLocally = (worker.getNrLocalBuilds() < worker.settings.maxBuildJobs) && !worker.settings.avoidLocal;
1211+
12011212
/* Send the request to the hook. */
1202-
worker.hook->sink << "try" << (worker.getNrLocalBuilds() < worker.settings.maxBuildJobs ? 1 : 0)
1213+
worker.hook->sink << "try" << (canBuildLocally ? 1 : 0)
12031214
<< drv->platform << worker.store.printStorePath(drvPath)
12041215
<< drvOptions.getRequiredSystemFeatures(*drv);
12051216
worker.hook->sink.flush();

src/libstore/build/derivation-goal.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ Goal::Co DerivationGoal::haveDerivation(bool storeDerivation)
7575
auto checkResult = checkPathValidity();
7676

7777
/* If they are all valid, then we're done. */
78-
if (checkResult && checkResult->second == PathStatus::Valid && buildMode == bmNormal) {
78+
if (checkResult && checkResult->second == PathStatus::Valid && (buildMode == bmNormal || buildMode == bmLocal)) {
7979
co_return doneSuccess(BuildResult::Success::AlreadyValid, checkResult->first);
8080
}
8181

@@ -134,7 +134,7 @@ Goal::Co DerivationGoal::haveDerivation(bool storeDerivation)
134134

135135
bool allValid = checkResult && checkResult->second == PathStatus::Valid;
136136

137-
if (buildMode == bmNormal && allValid) {
137+
if ((buildMode == bmNormal || buildMode == bmLocal) && allValid) {
138138
co_return doneSuccess(BuildResult::Success::Substituted, checkResult->first);
139139
}
140140
if (buildMode == bmRepair && allValid) {

src/libstore/include/nix/store/machines.hh

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ struct Machine
2323
const StringSet supportedFeatures;
2424
const StringSet mandatoryFeatures;
2525
const std::string sshPublicHostKey;
26+
const bool forceLocal;
2627
bool enabled = true;
2728

2829
/**
@@ -50,7 +51,8 @@ struct Machine
5051
decltype(speedFactor) speedFactor,
5152
decltype(supportedFeatures) supportedFeatures,
5253
decltype(mandatoryFeatures) mandatoryFeatures,
53-
decltype(sshPublicHostKey) sshPublicHostKey);
54+
decltype(sshPublicHostKey) sshPublicHostKey,
55+
decltype(forceLocal) forceLocal);
5456

5557
/**
5658
* Elaborate `storeUri` into a complete store reference,

src/libstore/include/nix/store/store-api.hh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ enum CheckSigsFlag : bool { NoCheckSigs = false, CheckSigs = true };
5050

5151
enum SubstituteFlag : bool { NoSubstitute = false, Substitute = true };
5252

53-
enum BuildMode : uint8_t { bmNormal, bmRepair, bmCheck };
53+
enum BuildMode : uint8_t { bmNormal, bmRepair, bmCheck, bmLocal };
5454

5555
enum TrustedFlag : bool { NotTrusted = false, Trusted = true };
5656

src/libstore/include/nix/store/worker-settings.hh

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,8 @@ public:
198198
199199
The value for this field can be obtained via `base64 -w0`.
200200
201+
9. A boolean which if true, forces the build to not use builders on the remote store
202+
201203
> **Example**
202204
>
203205
> Multiple builders specified on the command line:
@@ -270,6 +272,14 @@ public:
270272
This can drastically reduce build times if the network connection between the local machine and the remote build host is slow.
271273
)"};
272274

275+
Setting<bool> avoidLocal {
276+
this,
277+
false,
278+
"avoid-local",
279+
R"(
280+
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.
281+
)"};
282+
273283
Setting<bool> useSubstitutes{
274284
this,
275285
true,

src/libstore/machines.cc

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ Machine::Machine(
1414
decltype(speedFactor) speedFactor,
1515
decltype(supportedFeatures) supportedFeatures,
1616
decltype(mandatoryFeatures) mandatoryFeatures,
17-
decltype(sshPublicHostKey) sshPublicHostKey)
17+
decltype(sshPublicHostKey) sshPublicHostKey,
18+
decltype(forceLocal) forceLocal)
1819
: storeUri(
1920
StoreReference::parse(
2021
// Backwards compatibility: if the URI is schemeless, is not a path,
@@ -32,6 +33,7 @@ Machine::Machine(
3233
, supportedFeatures(supportedFeatures)
3334
, mandatoryFeatures(mandatoryFeatures)
3435
, sshPublicHostKey(sshPublicHostKey)
36+
, forceLocal(forceLocal)
3537
{
3638
if (speedFactor < 0.0)
3739
throw UsageError("speed factor must be >= 0");
@@ -188,7 +190,9 @@ static Machine parseBuilderLine(const StringSet & defaultSystems, const std::str
188190
// `mandatoryFeatures`
189191
isSet(6) ? tokenizeString<StringSet>(tokens[6], ",") : StringSet{},
190192
// `sshPublicHostKey`
191-
isSet(7) ? ensureBase64(7) : ""};
193+
isSet(7) ? ensureBase64(7) : "",
194+
// `forceLocal`
195+
isSet(8) ? (tokens[8] == "true") : false};
192196
}
193197

194198
static Machines parseBuilderLines(const StringSet & defaultSystems, const std::vector<std::string> & builders)

src/libstore/unix/build/chroot-derivation-builder.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ struct ChrootDerivationBuilder : virtual DerivationBuilderImpl
114114

115115
/* Move paths out of the chroot for easier debugging of
116116
build failures. */
117-
if (!force && buildMode == bmNormal)
117+
if (!force && (buildMode == bmNormal || buildMode == bmLocal))
118118
for (auto & [_, status] : initialOutputs) {
119119
if (!status.known)
120120
continue;

src/libstore/worker-protocol.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ BuildMode WorkerProto::Serialise<BuildMode>::read(const StoreDirConfig & store,
6767
return bmRepair;
6868
case 2:
6969
return bmCheck;
70+
case 3:
71+
return bmLocal;
7072
default:
7173
throw Error("Invalid build mode");
7274
}
@@ -85,6 +87,9 @@ void WorkerProto::Serialise<BuildMode>::write(
8587
case bmCheck:
8688
conn.to << uint8_t{2};
8789
break;
90+
case bmLocal:
91+
conn.to << uint8_t{3};
92+
break;
8893
default:
8994
assert(false);
9095
};

src/nix/build-remote/build-remote.cc

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,10 @@ static int main_build_remote(int argc, char ** argv)
110110
std::shared_ptr<Store> sshStore;
111111
AutoCloseFD bestSlotLock;
112112

113+
// Whether to force local building on the target store bypassing build
114+
// hooks
115+
bool forceLocal = false;
116+
113117
auto machines = Machine::parseConfig({settings.thisSystem}, settings.getWorkerSettings().builders);
114118
debug("got %d remote builders", machines.size());
115119

@@ -252,6 +256,8 @@ static int main_build_remote(int argc, char ** argv)
252256

253257
sshStore = bestMachine->openStore();
254258
sshStore->connect();
259+
260+
forceLocal = bestMachine->forceLocal;
255261
} catch (std::exception & e) {
256262
auto msg = chomp(drainFD(5, {.block = false}));
257263
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)
321327
!trusted || *trusted;
322328
});
323329

330+
auto buildMode = forceLocal ? bmLocal : bmNormal;
331+
324332
// See the very large comment in `case WorkerProto::Op::BuildDerivation:` in
325333
// `src/libstore/daemon.cc` that explains the trust model here.
326334
//
@@ -338,7 +346,7 @@ static int main_build_remote(int argc, char ** argv)
338346
// output ids, which break CA derivations
339347
if (!drv.inputDrvs.map.empty())
340348
drv.inputSrcs = store->parseStorePathSet(inputs);
341-
optResult = sshStore->buildDerivation(*drvPath, static_cast<const BasicDerivation &>(drv));
349+
optResult = sshStore->buildDerivation(*drvPath, static_cast<const BasicDerivation &>(drv), buildMode);
342350
auto & result = *optResult;
343351
if (auto * failureP = result.tryGetFailure()) {
344352
if (settings.keepFailed) {
@@ -356,7 +364,7 @@ static int main_build_remote(int argc, char ** argv)
356364
auto res = sshStore->buildPathsWithResults({DerivedPath::Built{
357365
.drvPath = makeConstantStorePathRef(*drvPath),
358366
.outputs = OutputsSpec::All{},
359-
}});
367+
}}, buildMode);
360368
// One path to build should produce exactly one build result
361369
assert(res.size() == 1);
362370
optResult = std::move(res[0]);

0 commit comments

Comments
 (0)