From 70e5469303b422bdb4b123be222bdea4d7f9611c Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 23 Jan 2024 12:08:05 -0500 Subject: [PATCH] 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