From 2e6ee28f9bef00ef08e6b6125f04af82cfca9fd3 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 23 Jan 2024 11:03:19 -0500 Subject: [PATCH 1/2] `Machine` -> `::Machine` so we don't conflict with Nix's --- src/hydra-queue-runner/build-remote.cc | 20 ++++++++++---------- src/hydra-queue-runner/builder.cc | 2 +- src/hydra-queue-runner/dispatcher.cc | 4 ++-- src/hydra-queue-runner/hydra-queue-runner.cc | 8 ++++---- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index a4f3a35c..5cc994fc 100644 --- a/src/hydra-queue-runner/build-remote.cc +++ b/src/hydra-queue-runner/build-remote.cc @@ -48,7 +48,7 @@ static Strings extraStoreArgs(std::string & machine) return result; } -static void openConnection(Machine::ptr machine, Path tmpDir, int stderrFD, SSHMaster::Connection & child) +static void openConnection(::Machine::ptr machine, Path tmpDir, int stderrFD, SSHMaster::Connection & child) { std::string pgmName; Pipe to, from; @@ -104,7 +104,7 @@ static void openConnection(Machine::ptr machine, Path tmpDir, int stderrFD, SSHM static void copyClosureTo( - Machine::Connection & conn, + ::Machine::Connection & conn, Store & destStore, const StorePathSet & paths, SubstituteFlag useSubstitutes = NoSubstitute) @@ -195,7 +195,7 @@ static std::pair openLogFile(const std::string & logDir, cons * Therefore, no `ServeProto::Serialize` functions can be used until * that field is set. */ -static void handshake(Machine::Connection & conn, unsigned int repeats) +static void handshake(::Machine::Connection & conn, unsigned int repeats) { conn.to << SERVE_MAGIC_1 << 0x206; conn.to.flush(); @@ -216,7 +216,7 @@ static BasicDerivation sendInputs( Step & step, Store & localStore, Store & destStore, - Machine::Connection & conn, + ::Machine::Connection & conn, unsigned int & overhead, counter & nrStepsWaiting, counter & nrStepsCopyingTo @@ -272,7 +272,7 @@ static BasicDerivation sendInputs( } static BuildResult performBuild( - Machine::Connection & conn, + ::Machine::Connection & conn, Store & localStore, StorePath drvPath, const BasicDerivation & drv, @@ -317,7 +317,7 @@ static BuildResult performBuild( } static std::map queryPathInfos( - Machine::Connection & conn, + ::Machine::Connection & conn, Store & localStore, StorePathSet & outputs, size_t & totalNarSize @@ -355,7 +355,7 @@ static std::map queryPathInfos( } static void copyPathFromRemote( - Machine::Connection & conn, + ::Machine::Connection & conn, NarMemberDatas & narMembers, Store & localStore, Store & destStore, @@ -385,7 +385,7 @@ static void copyPathFromRemote( } static void copyPathsFromRemote( - Machine::Connection & conn, + ::Machine::Connection & conn, NarMemberDatas & narMembers, Store & localStore, Store & destStore, @@ -462,7 +462,7 @@ void RemoteResult::updateWithBuildResult(const nix::BuildResult & buildResult) void State::buildRemote(ref destStore, - Machine::ptr machine, Step::ptr step, + ::Machine::ptr machine, Step::ptr step, const BuildOptions & buildOptions, RemoteResult & result, std::shared_ptr activeStep, std::function updateStep, @@ -503,7 +503,7 @@ void State::buildRemote(ref destStore, process. Meh. */ }); - Machine::Connection conn { + ::Machine::Connection conn { .from = child.out.get(), .to = child.in.get(), .machine = machine, diff --git a/src/hydra-queue-runner/builder.cc b/src/hydra-queue-runner/builder.cc index 307eee8e..cb343d77 100644 --- a/src/hydra-queue-runner/builder.cc +++ b/src/hydra-queue-runner/builder.cc @@ -400,7 +400,7 @@ void State::failStep( Step::ptr step, BuildID buildId, const RemoteResult & result, - Machine::ptr machine, + ::Machine::ptr machine, bool & stepFinished) { /* Register failure in the database for all Build objects that diff --git a/src/hydra-queue-runner/dispatcher.cc b/src/hydra-queue-runner/dispatcher.cc index af21dcbd..6d738ded 100644 --- a/src/hydra-queue-runner/dispatcher.cc +++ b/src/hydra-queue-runner/dispatcher.cc @@ -199,7 +199,7 @@ system_time State::doDispatch() filter out temporarily disabled machines. */ struct MachineInfo { - Machine::ptr machine; + ::Machine::ptr machine; unsigned long currentJobs; }; std::vector machinesSorted; @@ -435,7 +435,7 @@ void Jobset::pruneSteps() } -State::MachineReservation::MachineReservation(State & state, Step::ptr step, Machine::ptr machine) +State::MachineReservation::MachineReservation(State & state, Step::ptr step, ::Machine::ptr machine) : state(state), step(step), machine(machine) { machine->state->currentJobs++; diff --git a/src/hydra-queue-runner/hydra-queue-runner.cc b/src/hydra-queue-runner/hydra-queue-runner.cc index 1d54bb93..09d6fcb2 100644 --- a/src/hydra-queue-runner/hydra-queue-runner.cc +++ b/src/hydra-queue-runner/hydra-queue-runner.cc @@ -140,7 +140,7 @@ void State::parseMachines(const std::string & contents) if (tokens.size() < 3) continue; tokens.resize(8); - auto machine = std::make_shared(); + auto machine = std::make_shared<::Machine>(); machine->sshName = tokens[0]; machine->systemTypes = tokenizeString(tokens[1], ","); machine->sshKey = tokens[2] == "-" ? std::string("") : tokens[2]; @@ -166,7 +166,7 @@ void State::parseMachines(const std::string & contents) else printMsg(lvlChatty, "updating machine ‘%1%’", machine->sshName); machine->state = i == oldMachines.end() - ? std::make_shared() + ? std::make_shared<::Machine::State>() : i->second->state; newMachines[machine->sshName] = machine; } @@ -175,9 +175,9 @@ void State::parseMachines(const std::string & contents) if (newMachines.find(m.first) == newMachines.end()) { if (m.second->enabled) printInfo("removing machine ‘%1%’", m.first); - /* Add a disabled Machine object to make sure stats are + /* Add a disabled ::Machine object to make sure stats are maintained. */ - auto machine = std::make_shared(*(m.second)); + auto machine = std::make_shared<::Machine>(*(m.second)); machine->enabled = false; newMachines[m.first] = machine; } From 70e5469303b422bdb4b123be222bdea4d7f9611c Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 23 Jan 2024 12:08:05 -0500 Subject: [PATCH 2/2] Use Nix's `Machine` type in a mimimal way This is *just* using the fields from that type, and only where the types coincide. (There are two fields with different types, `speedFactor` most interestingly.) No code is reused, so we can be sure that no behavior is changed. Once the types are reconciled on the Nix side, then we can start carefully actually reusing code. Progress on #1164 --- src/hydra-queue-runner/dispatcher.cc | 6 +-- src/hydra-queue-runner/hydra-queue-runner.cc | 53 ++++++++++++++------ src/hydra-queue-runner/state.hh | 21 +++++--- 3 files changed, 53 insertions(+), 27 deletions(-) diff --git a/src/hydra-queue-runner/dispatcher.cc b/src/hydra-queue-runner/dispatcher.cc index 6d738ded..f5d9d3e3 100644 --- a/src/hydra-queue-runner/dispatcher.cc +++ b/src/hydra-queue-runner/dispatcher.cc @@ -231,11 +231,11 @@ system_time State::doDispatch() sort(machinesSorted.begin(), machinesSorted.end(), [](const MachineInfo & a, const MachineInfo & b) -> bool { - float ta = std::round(a.currentJobs / a.machine->speedFactor); - float tb = std::round(b.currentJobs / b.machine->speedFactor); + float ta = std::round(a.currentJobs / a.machine->speedFactorFloat); + float tb = std::round(b.currentJobs / b.machine->speedFactorFloat); return ta != tb ? ta < tb : - a.machine->speedFactor != b.machine->speedFactor ? a.machine->speedFactor > b.machine->speedFactor : + a.machine->speedFactorFloat != b.machine->speedFactorFloat ? a.machine->speedFactorFloat > b.machine->speedFactorFloat : a.currentJobs > b.currentJobs; }); diff --git a/src/hydra-queue-runner/hydra-queue-runner.cc b/src/hydra-queue-runner/hydra-queue-runner.cc index 09d6fcb2..66528302 100644 --- a/src/hydra-queue-runner/hydra-queue-runner.cc +++ b/src/hydra-queue-runner/hydra-queue-runner.cc @@ -1,6 +1,7 @@ #include #include #include +#include #include #include @@ -140,23 +141,43 @@ void State::parseMachines(const std::string & contents) if (tokens.size() < 3) continue; tokens.resize(8); - auto machine = std::make_shared<::Machine>(); - machine->sshName = tokens[0]; - machine->systemTypes = tokenizeString(tokens[1], ","); - machine->sshKey = tokens[2] == "-" ? std::string("") : tokens[2]; - if (tokens[3] != "") - machine->maxJobs = string2IntmaxJobs)>(tokens[3]).value(); - else - machine->maxJobs = 1; - machine->speedFactor = atof(tokens[4].c_str()); if (tokens[5] == "-") tokens[5] = ""; - machine->supportedFeatures = tokenizeString(tokens[5], ","); + auto supportedFeatures = tokenizeString(tokens[5], ","); + if (tokens[6] == "-") tokens[6] = ""; - machine->mandatoryFeatures = tokenizeString(tokens[6], ","); - for (auto & f : machine->mandatoryFeatures) - machine->supportedFeatures.insert(f); - if (tokens[7] != "" && tokens[7] != "-") - machine->sshPublicHostKey = base64Decode(tokens[7]); + auto mandatoryFeatures = tokenizeString(tokens[6], ","); + + for (auto & f : mandatoryFeatures) + supportedFeatures.insert(f); + + using MaxJobs = std::remove_const::type; + + auto machine = std::make_shared<::Machine>(nix::Machine { + // `storeUri`, not yet used + "", + // `systemTypes`, not yet used + {}, + // `sshKey` + tokens[2] == "-" ? "" : tokens[2], + // `maxJobs` + tokens[3] != "" + ? string2Int(tokens[3]).value() + : 1, + // `speedFactor`, not yet used + 1, + // `supportedFeatures` + std::move(supportedFeatures), + // `mandatoryFeatures` + std::move(mandatoryFeatures), + // `sshPublicHostKey` + tokens[7] != "" && tokens[7] != "-" + ? base64Decode(tokens[7]) + : "", + }); + + machine->sshName = tokens[0]; + machine->systemTypesSet = tokenizeString(tokens[1], ","); + machine->speedFactorFloat = atof(tokens[4].c_str()); /* Re-use the State object of the previous machine with the same name. */ @@ -596,7 +617,7 @@ void State::dumpStatus(Connection & conn) json machine = { {"enabled", m->enabled}, - {"systemTypes", m->systemTypes}, + {"systemTypes", m->systemTypesSet}, {"supportedFeatures", m->supportedFeatures}, {"mandatoryFeatures", m->mandatoryFeatures}, {"nrStepsDone", s->nrStepsDone.load()}, diff --git a/src/hydra-queue-runner/state.hh b/src/hydra-queue-runner/state.hh index 6359063a..b08e4e37 100644 --- a/src/hydra-queue-runner/state.hh +++ b/src/hydra-queue-runner/state.hh @@ -22,6 +22,7 @@ #include "sync.hh" #include "nar-extractor.hh" #include "serve-protocol.hh" +#include "machines.hh" typedef unsigned int BuildID; @@ -234,17 +235,21 @@ void getDependents(Step::ptr step, std::set & builds, std::set visitor, Step::ptr step); -struct Machine +struct Machine : nix::Machine { typedef std::shared_ptr ptr; - bool enabled{true}; + /* TODO Get rid of: `nix::Machine::storeUri` is normalized in a way + we are not yet used to, but once we are, we don't need this. */ + std::string sshName; - std::string sshName, sshKey; - std::set systemTypes, supportedFeatures, mandatoryFeatures; - unsigned int maxJobs = 1; - float speedFactor = 1.0; - std::string sshPublicHostKey; + /* TODO Get rid once `nix::Machine::systemTypes` is a set not + vector. */ + std::set systemTypesSet; + + /* TODO Get rid once `nix::Machine::systemTypes` is a `float` not + an `int`. */ + float speedFactorFloat = 1.0; struct State { typedef std::shared_ptr ptr; @@ -272,7 +277,7 @@ struct Machine { /* Check that this machine is of the type required by the step. */ - if (!systemTypes.count(step->drv->platform == "builtin" ? nix::settings.thisSystem : step->drv->platform)) + if (!systemTypesSet.count(step->drv->platform == "builtin" ? nix::settings.thisSystem : step->drv->platform)) return false; /* Check that the step requires all mandatory features of this