From 909542e1cbaa4f04d1f69352e4d88a90475220e6 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sun, 6 Apr 2025 02:21:19 +0000 Subject: [PATCH] Use ref for localStore/destStore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As suggested by pennae. The stores don't need to be nullable, so we initialize them right in the constructor of `State`. (cherry picked from commit 4b2720816d24c2e9770d1d65b73dbc8974c6dc1d) The lix changes 7bf3d0fa2aac73a838ee91e1170034dfe3722159...949a5615405d24294abab4d3dfb05958d87c32d6 have caused build errors which are addressed with the changes to hydra herein as well as additional changes in lix. Fixes: #36 Flake lock file updates: • Updated input 'lix': 'git+https://git.lix.systems/lix-project/lix?ref=refs/heads/main&rev=b918f1c307b314daa44407bac60046630f13ed48' (2025-03-27) → 'git+https://git.lix.systems/lix-project/lix?ref=refs/heads/main&rev=2ef4b69760af183792a740f425eb371a6aeb0009' (2025-04-06) Co-authored-by: benaryorg Signed-off-by: benaryorg --- flake.lock | 8 +++---- src/hydra-queue-runner/build-remote.cc | 2 +- src/hydra-queue-runner/builder.cc | 2 +- src/hydra-queue-runner/hydra-queue-runner.cc | 24 +++++++++++--------- src/hydra-queue-runner/state.hh | 6 ++--- t/scripts/hydra-send-stats.t | 7 ++++++ 6 files changed, 29 insertions(+), 20 deletions(-) diff --git a/flake.lock b/flake.lock index 71361d4e2..37216d1a8 100644 --- a/flake.lock +++ b/flake.lock @@ -28,11 +28,11 @@ "pre-commit-hooks": "pre-commit-hooks" }, "locked": { - "lastModified": 1743065774, - "narHash": "sha256-aC1lwnqwHbYRsPfx+lRi32fHRcln0xhIzf0Q7/Lg+vQ=", + "lastModified": 1743964165, + "narHash": "sha256-5vXjK5W+RIveud/Qn4OyVqrZi4pUDefKZsny33dDj+s=", "ref": "refs/heads/main", - "rev": "b918f1c307b314daa44407bac60046630f13ed48", - "revCount": 17742, + "rev": "2ef4b69760af183792a740f425eb371a6aeb0009", + "revCount": 17767, "type": "git", "url": "https://git.lix.systems/lix-project/lix" }, diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index b2cfca1a7..fff17e30f 100644 --- a/src/hydra-queue-runner/build-remote.cc +++ b/src/hydra-queue-runner/build-remote.cc @@ -654,7 +654,7 @@ void State::buildRemote(AsyncIoRoot & aio, outputs.insert(realisation.outPath); /* Copy the output paths. */ - if (!machine->isLocalhost() || localStore != std::shared_ptr(destStore)) { + if (!machine->isLocalhost() || localStore != destStore) { updateStep(ssReceivingOutputs); MaintainCount mc(nrStepsCopyingFrom); diff --git a/src/hydra-queue-runner/builder.cc b/src/hydra-queue-runner/builder.cc index 6243f315f..082cd237e 100644 --- a/src/hydra-queue-runner/builder.cc +++ b/src/hydra-queue-runner/builder.cc @@ -183,7 +183,7 @@ State::StepResult State::doBuildStep(AsyncIoRoot & aio, /* Upload the log file to the binary cache. FIXME: should be done on a worker thread. */ try { - auto store = destStore.dynamic_pointer_cast(); + auto store = destStore.try_cast_shared(); if (uploadLogsToBinaryCache && store && pathExists(result.logFile)) { store->upsertFile("log/" + std::string(step->drvPath.to_string()), readFile(result.logFile), "text/plain; charset=utf-8"); unlink(result.logFile.c_str()); diff --git a/src/hydra-queue-runner/hydra-queue-runner.cc b/src/hydra-queue-runner/hydra-queue-runner.cc index 070561020..42e82fc21 100644 --- a/src/hydra-queue-runner/hydra-queue-runner.cc +++ b/src/hydra-queue-runner/hydra-queue-runner.cc @@ -102,11 +102,21 @@ State::PromMetrics::PromMetrics() } -State::State(std::optional metricsAddrOpt) +State::State(std::optional metricsAddrOpt, AsyncIoRoot & aio) : config(std::make_unique()) , maxUnsupportedTime(config->getIntOption("max_unsupported_time", 0)) , dbPool(config->getIntOption("max_db_connections", 128)) , localWorkThrottler(static_cast(config->getIntOption("max_local_worker_threads", std::min(maxSupportedLocalWorkers, std::max(4u, std::thread::hardware_concurrency()) - 2)))) + , localStore([&]() { + StoreConfig::Params localParams; + localParams["max-connections"] = "16"; + localParams["max-connection-age"] = "600"; + return aio.blockOn(openStore(getEnv("NIX_REMOTE").value_or(""), localParams)); + }()) + , _destStore([&](){ + auto storeUri = config->getStrOption("store_uri"); + return storeUri == "" ? ref{*localStore} : aio.blockOn(openStore(storeUri)); + }()) , maxOutputSize(config->getIntOption("max_output_size", 2ULL << 30)) , maxLogSize(config->getIntOption("max_log_size", 64ULL << 20)) , uploadLogsToBinaryCache(config->getBoolOption("upload_logs_to_binary_cache", false)) @@ -145,7 +155,7 @@ nix::MaintainCount State::startDbUpdate() ref State::getDestStore() { - return ref(_destStore); + return ref{*_destStore}; } @@ -866,14 +876,6 @@ void State::run(AsyncIoRoot & aio, BuildID buildOne) << metricsAddr << "/metrics (port " << exposerPort << ")" << std::endl; - StoreConfig::Params localParams; - localParams["max-connections"] = "16"; - localParams["max-connection-age"] = "600"; - localStore = aio.blockOn(openStore(getEnv("NIX_REMOTE").value_or(""), localParams)); - - auto storeUri = config->getStrOption("store_uri"); - _destStore = storeUri == "" ? localStore : aio.blockOn(openStore(storeUri)); - useSubstitutes = config->getBoolOption("use-substitutes", false); // FIXME: hacky mechanism for configuring determinism checks. @@ -993,7 +995,7 @@ int main(int argc, char * * argv) settings.verboseBuild = true; - State state{metricsAddrOpt}; + State state{metricsAddrOpt, aio}; if (status) state.showStatus(aio); else if (unlock) diff --git a/src/hydra-queue-runner/state.hh b/src/hydra-queue-runner/state.hh index 94337948c..989b0956a 100644 --- a/src/hydra-queue-runner/state.hh +++ b/src/hydra-queue-runner/state.hh @@ -454,8 +454,8 @@ private: std::atomic lastDispatcherCheck{0}; - std::shared_ptr localStore; - std::shared_ptr _destStore; + nix::ref localStore; + nix::ref _destStore; size_t maxOutputSize; size_t maxLogSize; @@ -499,7 +499,7 @@ private: PromMetrics prom; public: - State(std::optional metricsAddrOpt); + State(std::optional metricsAddrOpt, nix::AsyncIoRoot & aio); struct BuildOptions { unsigned int maxSilentTime, buildTimeout, repeats; diff --git a/t/scripts/hydra-send-stats.t b/t/scripts/hydra-send-stats.t index dddeab1a2..68de0f066 100644 --- a/t/scripts/hydra-send-stats.t +++ b/t/scripts/hydra-send-stats.t @@ -16,6 +16,13 @@ hydra_setup($db); my ($res, $stdout, $stderr) = captureStdoutStderr(60, ("hydra-send-stats", "--once")); is($stdout, "", "hydra-send-stats stdout should be empty"); + +# Remove `unknown setting` warnings. These can happen in the dev shell with a +# local `nix.conf` or even in tests since the tests are using a local chroot store, +# but hydra-queue-runner sets store config for a remote store. +# hydra-queue-runner initializes stores in its ctor now, so these warnings get propagated +# in the tests to hydra-send-stats. +$stderr = join "\n", grep { 0 != rindex $_, "warning: unknown setting" } split "\n", $stderr; is($stderr, "", "hydra-send-stats stderr should be empty"); is($res, 0, "hydra-send-stats --once should exit zero");