From 90f03158f384876e38c89ffc8d29c558aaeb2560 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 27 Jul 2020 14:02:36 +0200 Subject: [PATCH 1/9] flake.lock: Update Flake input changes: * Updated 'nix': 'github:NixOS/nix/1ab9da915422405452118ebb17b88cdfc90b1e10' -> 'github:NixOS/nix/d7c0f094cbcfe1ae4ccc3d54baec00b66ccb1ed0' --- flake.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/flake.lock b/flake.lock index 4c0bbe4f..65f542f0 100644 --- a/flake.lock +++ b/flake.lock @@ -5,11 +5,11 @@ "nixpkgs": "nixpkgs" }, "locked": { - "lastModified": 1594125537, - "narHash": "sha256-M801IExREv1T9F+K6YcCFERBFZ3+6ShwzAR2K7xvExA=", + "lastModified": 1595761776, + "narHash": "sha256-Y678b0XLyYKqF98bpsmZzrnMz0W7hwGSEkZxDURpMFE=", "owner": "NixOS", "repo": "nix", - "rev": "1ab9da915422405452118ebb17b88cdfc90b1e10", + "rev": "d7c0f094cbcfe1ae4ccc3d54baec00b66ccb1ed0", "type": "github" }, "original": { From e5f6fc2e4efc322c8389346adcb3cf8f5245d274 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 27 Jul 2020 14:53:43 +0200 Subject: [PATCH 2/9] Quick hack to fix compilation --- src/hydra-queue-runner/build-remote.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index 831e4639..3724399a 100644 --- a/src/hydra-queue-runner/build-remote.cc +++ b/src/hydra-queue-runner/build-remote.cc @@ -444,7 +444,7 @@ void State::buildRemote(ref destStore, to << cmdExportPaths << 0; writeStorePaths(*localStore, to, outputs); to.flush(); - destStore->importPaths(from, result.accessor, NoCheckSigs); + destStore->importPaths(from, /* result.accessor, */ NoCheckSigs); /* Release the tokens pertaining to NAR compression. After this we only have the uncompressed From cbcf6359b4b8525ab6252de2c3cc389397183159 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 27 Jul 2020 14:57:16 +0200 Subject: [PATCH 3/9] Remove TokenServer in preparation of making NAR copying O(1) memory --- src/hydra-queue-runner/Makefile.am | 2 +- src/hydra-queue-runner/build-remote.cc | 20 ---- src/hydra-queue-runner/builder.cc | 3 - src/hydra-queue-runner/hydra-queue-runner.cc | 4 - src/hydra-queue-runner/state.hh | 9 -- src/hydra-queue-runner/token-server.hh | 109 ------------------- 6 files changed, 1 insertion(+), 146 deletions(-) delete mode 100644 src/hydra-queue-runner/token-server.hh diff --git a/src/hydra-queue-runner/Makefile.am b/src/hydra-queue-runner/Makefile.am index 1726d0df..9582a629 100644 --- a/src/hydra-queue-runner/Makefile.am +++ b/src/hydra-queue-runner/Makefile.am @@ -2,6 +2,6 @@ bin_PROGRAMS = hydra-queue-runner hydra_queue_runner_SOURCES = hydra-queue-runner.cc queue-monitor.cc dispatcher.cc \ builder.cc build-result.cc build-remote.cc \ - build-result.hh counter.hh token-server.hh state.hh db.hh + build-result.hh counter.hh state.hh db.hh hydra_queue_runner_LDADD = $(NIX_LIBS) -lpqxx hydra_queue_runner_CXXFLAGS = $(NIX_CFLAGS) -Wall -I ../libhydra -Wno-deprecated-declarations diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index 3724399a..320f6d16 100644 --- a/src/hydra-queue-runner/build-remote.cc +++ b/src/hydra-queue-runner/build-remote.cc @@ -426,31 +426,11 @@ void State::buildRemote(ref destStore, printMsg(lvlDebug, "copying outputs of ‘%s’ from ‘%s’ (%d bytes)", localStore->printStorePath(step->drvPath), machine->sshName, totalNarSize); - /* Block until we have the required amount of memory - available, which is twice the NAR size (namely the - uncompressed and worst-case compressed NAR), plus 150 - MB for xz compression overhead. (The xz manpage claims - ~94 MiB, but that's not was I'm seeing.) */ - auto resStart = std::chrono::steady_clock::now(); - size_t compressionCost = totalNarSize + 150 * 1024 * 1024; - result.tokens = std::make_unique(memoryTokens.get(totalNarSize + compressionCost)); - auto resStop = std::chrono::steady_clock::now(); - - auto resMs = std::chrono::duration_cast(resStop - resStart).count(); - if (resMs >= 1000) - printMsg(lvlError, "warning: had to wait %d ms for %d memory tokens for %s", - resMs, totalNarSize, localStore->printStorePath(step->drvPath)); - to << cmdExportPaths << 0; writeStorePaths(*localStore, to, outputs); to.flush(); destStore->importPaths(from, /* result.accessor, */ NoCheckSigs); - /* Release the tokens pertaining to NAR - compression. After this we only have the uncompressed - NAR in memory. */ - result.tokens->give_back(compressionCost); - auto now2 = std::chrono::steady_clock::now(); result.overhead += std::chrono::duration_cast(now2 - now1).count(); diff --git a/src/hydra-queue-runner/builder.cc b/src/hydra-queue-runner/builder.cc index 793f3cdc..bd1e64a7 100644 --- a/src/hydra-queue-runner/builder.cc +++ b/src/hydra-queue-runner/builder.cc @@ -204,8 +204,6 @@ State::StepResult State::doBuildStep(nix::ref destStore, try { /* FIXME: referring builds may have conflicting timeouts. */ buildRemote(destStore, machine, step, maxSilentTime, buildTimeout, repeats, result, activeStep, updateStep); - } catch (NoTokens & e) { - result.stepStatus = bsNarSizeLimitExceeded; } catch (Error & e) { if (activeStep->state_.lock()->cancelled) { printInfo("marking step %d of build %d as cancelled", stepNr, buildId); @@ -224,7 +222,6 @@ State::StepResult State::doBuildStep(nix::ref destStore, } result.accessor = 0; - result.tokens = 0; } time_t stepStopTime = time(0); diff --git a/src/hydra-queue-runner/hydra-queue-runner.cc b/src/hydra-queue-runner/hydra-queue-runner.cc index 073af7ff..52ed1b23 100644 --- a/src/hydra-queue-runner/hydra-queue-runner.cc +++ b/src/hydra-queue-runner/hydra-queue-runner.cc @@ -49,14 +49,11 @@ State::State() : config(std::make_unique()) , maxUnsupportedTime(config->getIntOption("max_unsupported_time", 0)) , dbPool(config->getIntOption("max_db_connections", 128)) - , memoryTokens(config->getIntOption("nar_buffer_size", getMemSize() / 2)) , 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)) , rootsDir(config->getStrOption("gc_roots_dir", fmt("%s/gcroots/per-user/%s/hydra-roots", settings.nixStateDir, getEnvOrDie("LOGNAME")))) { - debug("using %d bytes for the NAR buffer", memoryTokens.capacity()); - hydraData = getEnvOrDie("HYDRA_DATA"); logDir = canonPath(hydraData + "/build-logs"); @@ -544,7 +541,6 @@ void State::dumpStatus(Connection & conn) root.attr("dispatchTimeAvgMs", nrDispatcherWakeups == 0 ? 0.0 : (float) dispatchTimeMs / nrDispatcherWakeups); root.attr("nrDbConnections", dbPool.count()); root.attr("nrActiveDbUpdates", nrActiveDbUpdates); - root.attr("memoryTokensInUse", memoryTokens.currentUse()); { auto nested = root.object("machines"); diff --git a/src/hydra-queue-runner/state.hh b/src/hydra-queue-runner/state.hh index 2447a6ff..28364db0 100644 --- a/src/hydra-queue-runner/state.hh +++ b/src/hydra-queue-runner/state.hh @@ -8,7 +8,6 @@ #include #include "db.hh" -#include "token-server.hh" #include "parsed-derivations.hh" #include "pathlocks.hh" @@ -65,7 +64,6 @@ struct RemoteResult time_t startTime = 0, stopTime = 0; unsigned int overhead = 0; nix::Path logFile; - std::unique_ptr tokens; std::shared_ptr accessor; BuildStatus buildStatus() const @@ -410,13 +408,6 @@ private: std::shared_ptr localStore; std::shared_ptr _destStore; - /* Token server to prevent threads from allocating too many big - strings concurrently while importing NARs from the build - machines. When a thread imports a NAR of size N, it will first - acquire N memory tokens, causing it to block until that many - tokens are available. */ - nix::TokenServer memoryTokens; - size_t maxOutputSize; size_t maxLogSize; diff --git a/src/hydra-queue-runner/token-server.hh b/src/hydra-queue-runner/token-server.hh deleted file mode 100644 index f4ff5822..00000000 --- a/src/hydra-queue-runner/token-server.hh +++ /dev/null @@ -1,109 +0,0 @@ -#pragma once - -#include - -#include "sync.hh" -#include "types.hh" - -namespace nix { - -MakeError(NoTokens, Error); - -/* This class hands out tokens. There are only ‘maxTokens’ tokens - available. Calling get(N) will return a Token object, representing - ownership of N tokens. If the requested number of tokens is - unavailable, get() will sleep until another thread returns a - token. */ - -class TokenServer -{ - const size_t maxTokens; - - Sync inUse{0}; - std::condition_variable wakeup; - -public: - TokenServer(size_t maxTokens) : maxTokens(maxTokens) { } - - class Token - { - friend TokenServer; - - TokenServer * ts; - - size_t tokens; - - bool acquired = false; - - Token(TokenServer * ts, size_t tokens, unsigned int timeout) - : ts(ts), tokens(tokens) - { - if (tokens >= ts->maxTokens) - throw NoTokens("requesting more tokens (%d) than exist (%d)", tokens, ts->maxTokens); - debug("acquiring %d tokens", tokens); - auto inUse(ts->inUse.lock()); - while (*inUse + tokens > ts->maxTokens) - if (timeout) { - if (!inUse.wait_for(ts->wakeup, std::chrono::seconds(timeout), - [&]() { return *inUse + tokens <= ts->maxTokens; })) - return; - } else - inUse.wait(ts->wakeup); - *inUse += tokens; - acquired = true; - } - - public: - - Token(Token && t) : ts(t.ts), tokens(t.tokens), acquired(t.acquired) - { - t.ts = 0; - t.acquired = false; - } - Token(const Token & l) = delete; - - ~Token() - { - if (!ts || !acquired) return; - give_back(tokens); - } - - bool operator ()() { return acquired; } - - void give_back(size_t t) - { - debug("returning %d tokens", t); - if (!t) return; - assert(acquired); - assert(t <= tokens); - { - auto inUse(ts->inUse.lock()); - assert(*inUse >= t); - *inUse -= t; - tokens -= t; - } - // FIXME: inefficient. Should wake up waiters that can - // proceed now. - ts->wakeup.notify_all(); - } - - }; - - Token get(size_t tokens = 1, unsigned int timeout = 0) - { - return Token(this, tokens, timeout); - } - - size_t currentUse() - { - auto inUse_(inUse.lock()); - return *inUse_; - } - - size_t capacity() - { - return maxTokens; - } -}; - -} From 7622cbfe3706767a9e7635c1bd23c7f2a9eea4bf Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 27 Jul 2020 18:11:04 +0200 Subject: [PATCH 4/9] buildRemote(): Copy paths to the destination store in O(1) memory --- src/hydra-queue-runner/build-remote.cc | 65 ++++++++++++++++++++++---- 1 file changed, 56 insertions(+), 9 deletions(-) diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index 320f6d16..26d991ef 100644 --- a/src/hydra-queue-runner/build-remote.cc +++ b/src/hydra-queue-runner/build-remote.cc @@ -124,6 +124,38 @@ static void copyClosureTo(std::timed_mutex & sendMutex, ref destStore, } +// FIXME: use Store::topoSortPaths(). +StorePaths topoSortPaths(const std::map & paths) +{ + StorePaths sorted; + StorePathSet visited; + + std::function dfsVisit; + + dfsVisit = [&](const StorePath & path) { + if (!visited.insert(path).second) return; + + auto info = paths.find(path); + auto references = info == paths.end() ? StorePathSet() : info->second.references; + + for (auto & i : references) + /* Don't traverse into paths that don't exist. That can + happen due to substitutes for non-existent paths. */ + if (i != path && paths.count(i)) + dfsVisit(i); + + sorted.push_back(path); + }; + + for (auto & i : paths) + dfsVisit(i.first); + + std::reverse(sorted.begin(), sorted.end()); + + return sorted; +} + + void State::buildRemote(ref destStore, Machine::ptr machine, Step::ptr step, unsigned int maxSilentTime, unsigned int buildTimeout, unsigned int repeats, @@ -148,6 +180,7 @@ void State::buildRemote(ref destStore, updateStep(ssConnecting); + // FIXME: rewrite to use Store. Child child; openConnection(machine, tmpDir, logFD.get(), child); @@ -182,7 +215,7 @@ void State::buildRemote(ref destStore, unsigned int remoteVersion; try { - to << SERVE_MAGIC_1 << 0x203; + to << SERVE_MAGIC_1 << 0x204; to.flush(); unsigned int magic = readInt(from); @@ -405,17 +438,26 @@ void State::buildRemote(ref destStore, auto outputs = step->drv->outputPaths(); - /* Query the size of the output paths. */ + /* Get info about each output path. */ + std::map infos; size_t totalNarSize = 0; to << cmdQueryPathInfos; writeStorePaths(*localStore, to, outputs); to.flush(); while (true) { - if (readString(from) == "") break; + auto storePathS = readString(from); + if (storePathS == "") break; + ValidPathInfo info(localStore->parseStorePath(storePathS)); + assert(outputs.count(info.path)); readString(from); // deriver - readStrings(from); // references + info.references = readStorePaths(*localStore, from); readLongLong(from); // download size - totalNarSize += readLongLong(from); + info.narSize = readLongLong(from); + totalNarSize += info.narSize; + info.narHash = Hash(readString(from), htSHA256); + info.ca = parseContentAddressOpt(readString(from)); + readStrings(from); // sigs + infos.insert_or_assign(info.path, info); } if (totalNarSize > maxOutputSize) { @@ -423,13 +465,18 @@ void State::buildRemote(ref destStore, return; } + /* Copy each path. */ printMsg(lvlDebug, "copying outputs of ‘%s’ from ‘%s’ (%d bytes)", localStore->printStorePath(step->drvPath), machine->sshName, totalNarSize); - to << cmdExportPaths << 0; - writeStorePaths(*localStore, to, outputs); - to.flush(); - destStore->importPaths(from, /* result.accessor, */ NoCheckSigs); + auto pathsSorted = topoSortPaths(infos); + + for (auto & path : pathsSorted) { + auto & info = infos.find(path)->second; + to << cmdDumpStorePath << localStore->printStorePath(path); + to.flush(); + destStore->addToStore(info, from); + } auto now2 = std::chrono::steady_clock::now(); From d4e4be4fd18d86826501e85e5f0195e49e2e3d1e Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 27 Jul 2020 18:24:10 +0200 Subject: [PATCH 5/9] Remove SHA-1 hash from BuildProducts SHA-1 is deprecated and it will be expensive to compute with the streaming NAR handler. --- doc/manual/api.xml | 1 - hydra-api.yaml | 5 ----- src/hydra-queue-runner/build-result.cc | 1 - src/hydra-queue-runner/build-result.hh | 2 +- src/hydra-queue-runner/hydra-queue-runner.cc | 3 +-- src/hydra-queue-runner/queue-monitor.cc | 14 ++++++-------- src/lib/Hydra/Plugin/RunCommand.pm | 1 - src/lib/Hydra/Schema/BuildProducts.pm | 12 ++---------- src/root/product-list.tt | 1 - src/sql/hydra.sql | 1 - src/sql/upgrade-68.sql | 1 + 11 files changed, 11 insertions(+), 31 deletions(-) create mode 100644 src/sql/upgrade-68.sql diff --git a/doc/manual/api.xml b/doc/manual/api.xml index 6656d98b..db5ba07e 100644 --- a/doc/manual/api.xml +++ b/doc/manual/api.xml @@ -311,7 +311,6 @@ Content-Type: application/json "buildproducts": { "1": { "path": "/nix/store/lzrxkjc35mhp8w7r8h82g0ljyizfchma-vm-test-run-unnamed", - "sha1hash": null, "defaultpath": "log.html", "type": "report", "sha256hash": null, diff --git a/hydra-api.yaml b/hydra-api.yaml index 5a4e36a1..7da99eb8 100644 --- a/hydra-api.yaml +++ b/hydra-api.yaml @@ -672,10 +672,6 @@ components: name: description: Name of the file type: string - sha1hash: - nullable: true - description: sha1 hash of the file - type: string path: description: the nix store path type: string @@ -863,7 +859,6 @@ components: defaultpath: '' name: hello-2.10 type: nix-build - sha1hash: null sha256hash: null subtype: '' path: /nix/store/y26qxcq1gg2hrqpxdc58b2fghv2bhxjg-hello-2.10 diff --git a/src/hydra-queue-runner/build-result.cc b/src/hydra-queue-runner/build-result.cc index f11d0a7c..868ccb15 100644 --- a/src/hydra-queue-runner/build-result.cc +++ b/src/hydra-queue-runner/build-result.cc @@ -78,7 +78,6 @@ BuildOutput getBuildOutput(nix::ref store, product.isRegular = true; product.fileSize = st.fileSize; auto contents = accessor->readFile(product.path); - product.sha1hash = hashString(htSHA1, contents); product.sha256hash = hashString(htSHA256, contents); } diff --git a/src/hydra-queue-runner/build-result.hh b/src/hydra-queue-runner/build-result.hh index 72e8b4df..57676797 100644 --- a/src/hydra-queue-runner/build-result.hh +++ b/src/hydra-queue-runner/build-result.hh @@ -11,7 +11,7 @@ struct BuildProduct nix::Path path, defaultPath; std::string type, subtype, name; bool isRegular = false; - nix::Hash sha1hash, sha256hash; + nix::Hash sha256hash; off_t fileSize = 0; BuildProduct() { } }; diff --git a/src/hydra-queue-runner/hydra-queue-runner.cc b/src/hydra-queue-runner/hydra-queue-runner.cc index 52ed1b23..0da14bcd 100644 --- a/src/hydra-queue-runner/hydra-queue-runner.cc +++ b/src/hydra-queue-runner/hydra-queue-runner.cc @@ -414,13 +414,12 @@ void State::markSucceededBuild(pqxx::work & txn, Build::ptr build, unsigned int productNr = 1; for (auto & product : res.products) { txn.exec_params0 - ("insert into BuildProducts (build, productnr, type, subtype, fileSize, sha1hash, sha256hash, path, name, defaultPath) values ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10)", + ("insert into BuildProducts (build, productnr, type, subtype, fileSize, sha256hash, path, name, defaultPath) values ($1, $2, $3, $4, $5, $6, $7, $8, $9)", build->id, productNr++, product.type, product.subtype, product.isRegular ? std::make_optional(product.fileSize) : std::nullopt, - product.isRegular ? std::make_optional(product.sha1hash.to_string(Base16, false)) : std::nullopt, product.isRegular ? std::make_optional(product.sha256hash.to_string(Base16, false)) : std::nullopt, product.path, product.name, diff --git a/src/hydra-queue-runner/queue-monitor.cc b/src/hydra-queue-runner/queue-monitor.cc index 24f4d04e..5bfcb1eb 100644 --- a/src/hydra-queue-runner/queue-monitor.cc +++ b/src/hydra-queue-runner/queue-monitor.cc @@ -632,7 +632,7 @@ BuildOutput State::getBuildOutputCached(Connection & conn, nix::ref res.size = r[0][4].is_null() ? 0 : r[0][4].as(); auto products = txn.exec_params - ("select type, subtype, fileSize, sha1hash, sha256hash, path, name, defaultPath from BuildProducts where build = $1 order by productnr", + ("select type, subtype, fileSize, sha256hash, path, name, defaultPath from BuildProducts where build = $1 order by productnr", id); for (auto row : products) { @@ -646,14 +646,12 @@ BuildOutput State::getBuildOutputCached(Connection & conn, nix::ref product.fileSize = row[2].as(); } if (!row[3].is_null()) - product.sha1hash = Hash(row[3].as(), htSHA1); + product.sha256hash = Hash(row[3].as(), htSHA256); if (!row[4].is_null()) - product.sha256hash = Hash(row[4].as(), htSHA256); - if (!row[5].is_null()) - product.path = row[5].as(); - product.name = row[6].as(); - if (!row[7].is_null()) - product.defaultPath = row[7].as(); + product.path = row[4].as(); + product.name = row[5].as(); + if (!row[6].is_null()) + product.defaultPath = row[6].as(); res.products.emplace_back(product); } diff --git a/src/lib/Hydra/Plugin/RunCommand.pm b/src/lib/Hydra/Plugin/RunCommand.pm index f7ca45a9..8ae70c6f 100644 --- a/src/lib/Hydra/Plugin/RunCommand.pm +++ b/src/lib/Hydra/Plugin/RunCommand.pm @@ -90,7 +90,6 @@ sub buildFinished { type => $product->type, subtype => $product->subtype, fileSize => $product->filesize, - sha1hash => $product->sha1hash, sha256hash => $product->sha256hash, path => $product->path, name => $product->name, diff --git a/src/lib/Hydra/Schema/BuildProducts.pm b/src/lib/Hydra/Schema/BuildProducts.pm index f52b2937..788cc390 100644 --- a/src/lib/Hydra/Schema/BuildProducts.pm +++ b/src/lib/Hydra/Schema/BuildProducts.pm @@ -61,11 +61,6 @@ __PACKAGE__->table("buildproducts"); data_type: 'bigint' is_nullable: 1 -=head2 sha1hash - - data_type: 'text' - is_nullable: 1 - =head2 sha256hash data_type: 'text' @@ -99,8 +94,6 @@ __PACKAGE__->add_columns( { data_type => "text", is_nullable => 0 }, "filesize", { data_type => "bigint", is_nullable => 1 }, - "sha1hash", - { data_type => "text", is_nullable => 1 }, "sha256hash", { data_type => "text", is_nullable => 1 }, "path", @@ -143,8 +136,8 @@ __PACKAGE__->belongs_to( ); -# Created by DBIx::Class::Schema::Loader v0.07049 @ 2020-02-06 12:22:36 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:iI0gmKqQxiPBTy5QsM6tpQ +# Created by DBIx::Class::Schema::Loader v0.07049 @ 2020-07-27 18:21:03 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:O4R8b/GukNaUmmAErb3Jlw my %hint = ( columns => [ @@ -152,7 +145,6 @@ my %hint = ( 'subtype', 'name', 'filesize', - 'sha1hash', 'sha256hash', 'path', 'defaultpath' diff --git a/src/root/product-list.tt b/src/root/product-list.tt index 298d0a66..efb3043b 100644 --- a/src/root/product-list.tt +++ b/src/root/product-list.tt @@ -191,7 +191,6 @@ [% INCLUDE renderProductLinks %] -
File size:[% product.filesize %] bytes ([% mibs(product.filesize / (1024 * 1024)) %] MiB)
SHA-1 hash:[% product.sha1hash %]
SHA-256 hash:[% product.sha256hash %]
Full path:[% product.path %]
diff --git a/src/sql/hydra.sql b/src/sql/hydra.sql index a0db622a..88c4c330 100644 --- a/src/sql/hydra.sql +++ b/src/sql/hydra.sql @@ -330,7 +330,6 @@ create table BuildProducts ( type text not null, -- "nix-build", "file", "doc", "report", ... subtype text not null, -- "source-dist", "rpm", ... fileSize bigint, - sha1hash text, sha256hash text, path text, name text not null, -- generally just the filename part of `path' diff --git a/src/sql/upgrade-68.sql b/src/sql/upgrade-68.sql new file mode 100644 index 00000000..368ed82b --- /dev/null +++ b/src/sql/upgrade-68.sql @@ -0,0 +1 @@ +alter table BuildProducts drop column sha1hash; From 5b4df3ad5a2b318a29b6bebc3e6be353b8c61d65 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 27 Jul 2020 20:38:59 +0200 Subject: [PATCH 6/9] Get data needed by getBuildOutput() from the incoming NAR in a streaming fashion --- src/hydra-queue-runner/Makefile.am | 5 +- src/hydra-queue-runner/build-remote.cc | 17 +++-- src/hydra-queue-runner/build-result.cc | 67 ++++++++++++-------- src/hydra-queue-runner/build-result.hh | 7 ++- src/hydra-queue-runner/builder.cc | 8 +-- src/hydra-queue-runner/nar-extractor.cc | 82 +++++++++++++++++++++++++ src/hydra-queue-runner/nar-extractor.hh | 23 +++++++ src/hydra-queue-runner/queue-monitor.cc | 3 +- src/hydra-queue-runner/state.hh | 5 +- 9 files changed, 177 insertions(+), 40 deletions(-) create mode 100644 src/hydra-queue-runner/nar-extractor.cc create mode 100644 src/hydra-queue-runner/nar-extractor.hh diff --git a/src/hydra-queue-runner/Makefile.am b/src/hydra-queue-runner/Makefile.am index 9582a629..ea852334 100644 --- a/src/hydra-queue-runner/Makefile.am +++ b/src/hydra-queue-runner/Makefile.am @@ -1,7 +1,8 @@ bin_PROGRAMS = hydra-queue-runner hydra_queue_runner_SOURCES = hydra-queue-runner.cc queue-monitor.cc dispatcher.cc \ - builder.cc build-result.cc build-remote.cc \ - build-result.hh counter.hh state.hh db.hh + builder.cc build-result.cc build-remote.cc \ + build-result.hh counter.hh state.hh db.hh \ + nar-extractor.cc nar-extractor.hh hydra_queue_runner_LDADD = $(NIX_LIBS) -lpqxx hydra_queue_runner_CXXFLAGS = $(NIX_CFLAGS) -Wall -I ../libhydra -Wno-deprecated-declarations diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index 26d991ef..b7d5ff85 100644 --- a/src/hydra-queue-runner/build-remote.cc +++ b/src/hydra-queue-runner/build-remote.cc @@ -160,7 +160,8 @@ void State::buildRemote(ref destStore, Machine::ptr machine, Step::ptr step, unsigned int maxSilentTime, unsigned int buildTimeout, unsigned int repeats, RemoteResult & result, std::shared_ptr activeStep, - std::function updateStep) + std::function updateStep, + NarMemberDatas & narMembers) { assert(BuildResult::TimedOut == 8); @@ -427,8 +428,6 @@ void State::buildRemote(ref destStore, } /* Copy the output paths. */ - result.accessor = destStore->getFSAccessor(); - if (!machine->isLocalhost() || localStore != std::shared_ptr(destStore)) { updateStep(ssReceivingOutputs); @@ -475,7 +474,17 @@ void State::buildRemote(ref destStore, auto & info = infos.find(path)->second; to << cmdDumpStorePath << localStore->printStorePath(path); to.flush(); - destStore->addToStore(info, from); + + /* Receive the NAR from the remote and add it to the + destination store. Meanwhile, extract all the info from the + NAR that getBuildOutput() needs. */ + auto source2 = sinkToSource([&](Sink & sink) + { + TeeSource tee(from, sink); + extractNarData(tee, localStore->printStorePath(path), narMembers); + }); + + destStore->addToStore(info, *source2); } auto now2 = std::chrono::steady_clock::now(); diff --git a/src/hydra-queue-runner/build-result.cc b/src/hydra-queue-runner/build-result.cc index 868ccb15..43af82d5 100644 --- a/src/hydra-queue-runner/build-result.cc +++ b/src/hydra-queue-runner/build-result.cc @@ -8,8 +8,10 @@ using namespace nix; -BuildOutput getBuildOutput(nix::ref store, - nix::ref accessor, const Derivation & drv) +BuildOutput getBuildOutput( + nix::ref store, + NarMemberDatas & narMembers, + const Derivation & drv) { BuildOutput res; @@ -24,6 +26,20 @@ BuildOutput getBuildOutput(nix::ref store, if (outputs.count(path)) res.size += info->narSize; } + /* Fetch missing data. Usually buildRemote() will have extracted + this data from the incoming NARs. */ + for (auto & output : outputs) { + auto outputS = store->printStorePath(output); + if (!narMembers.count(outputS)) { + printInfo("fetching NAR contents of '%s'...", outputS); + auto source = sinkToSource([&](Sink & sink) + { + store->narFromPath(output, sink); + }); + extractNarData(*source, outputS, narMembers); + } + } + /* Get build products. */ bool explicitProducts = false; @@ -39,17 +55,18 @@ BuildOutput getBuildOutput(nix::ref store, for (auto & output : outputs) { auto outputS = store->printStorePath(output); - Path failedFile = outputS + "/nix-support/failed"; - if (accessor->stat(failedFile).type == FSAccessor::Type::tRegular) + if (narMembers.count(outputS + "/nix-support/failed")) res.failed = true; - Path productsFile = outputS + "/nix-support/hydra-build-products"; - if (accessor->stat(productsFile).type != FSAccessor::Type::tRegular) + auto productsFile = narMembers.find(outputS + "/nix-support/hydra-build-products"); + if (productsFile == narMembers.end() || + productsFile->second.type != FSAccessor::Type::tRegular) continue; + assert(productsFile->second.contents); explicitProducts = true; - for (auto & line : tokenizeString(accessor->readFile(productsFile), "\n")) { + for (auto & line : tokenizeString(productsFile->second.contents.value(), "\n")) { BuildProduct product; std::smatch match; @@ -69,16 +86,15 @@ BuildOutput getBuildOutput(nix::ref store, product.path = canonPath(product.path); if (!store->isInStore(product.path)) continue; - auto st = accessor->stat(product.path); - if (st.type == FSAccessor::Type::tMissing) continue; + auto file = narMembers.find(product.path); + if (file == narMembers.end()) continue; product.name = product.path == store->printStorePath(output) ? "" : baseNameOf(product.path); - if (st.type == FSAccessor::Type::tRegular) { + if (file->second.type == FSAccessor::Type::tRegular) { product.isRegular = true; - product.fileSize = st.fileSize; - auto contents = accessor->readFile(product.path); - product.sha256hash = hashString(htSHA256, contents); + product.fileSize = file->second.fileSize.value(); + product.sha256hash = file->second.sha256.value(); } res.products.push_back(product); @@ -95,29 +111,30 @@ BuildOutput getBuildOutput(nix::ref store, product.subtype = output.first == "out" ? "" : output.first; product.name = output.second.path.name(); - auto st = accessor->stat(product.path); - if (st.type == FSAccessor::Type::tMissing) - throw Error("getting status of ‘%s’", product.path); - if (st.type == FSAccessor::Type::tDirectory) + auto file = narMembers.find(product.path); + assert(file != narMembers.end()); + if (file->second.type == FSAccessor::Type::tDirectory) res.products.push_back(product); } } /* Get the release name from $output/nix-support/hydra-release-name. */ for (auto & output : outputs) { - auto p = store->printStorePath(output) + "/nix-support/hydra-release-name"; - if (accessor->stat(p).type != FSAccessor::Type::tRegular) continue; - try { - res.releaseName = trim(accessor->readFile(p)); - } catch (Error & e) { continue; } + auto file = narMembers.find(store->printStorePath(output) + "/nix-support/hydra-release-name"); + if (file == narMembers.end() || + file->second.type != FSAccessor::Type::tRegular) + continue; + res.releaseName = trim(file->second.contents.value()); // FIXME: validate release name } /* Get metrics. */ for (auto & output : outputs) { - auto metricsFile = store->printStorePath(output) + "/nix-support/hydra-metrics"; - if (accessor->stat(metricsFile).type != FSAccessor::Type::tRegular) continue; - for (auto & line : tokenizeString(accessor->readFile(metricsFile), "\n")) { + auto file = narMembers.find(store->printStorePath(output) + "/nix-support/hydra-metrics"); + if (file == narMembers.end() || + file->second.type != FSAccessor::Type::tRegular) + continue; + for (auto & line : tokenizeString(file->second.contents.value(), "\n")) { auto fields = tokenizeString>(line); if (fields.size() < 2) continue; BuildMetric metric; diff --git a/src/hydra-queue-runner/build-result.hh b/src/hydra-queue-runner/build-result.hh index 57676797..a2b9a0f9 100644 --- a/src/hydra-queue-runner/build-result.hh +++ b/src/hydra-queue-runner/build-result.hh @@ -5,6 +5,7 @@ #include "hash.hh" #include "derivations.hh" #include "store-api.hh" +#include "nar-extractor.hh" struct BuildProduct { @@ -38,5 +39,7 @@ struct BuildOutput std::map metrics; }; -BuildOutput getBuildOutput(nix::ref store, - nix::ref accessor, const nix::Derivation & drv); +BuildOutput getBuildOutput( + nix::ref store, + NarMemberDatas & narMembers, + const nix::Derivation & drv); diff --git a/src/hydra-queue-runner/builder.cc b/src/hydra-queue-runner/builder.cc index bd1e64a7..16cc56a8 100644 --- a/src/hydra-queue-runner/builder.cc +++ b/src/hydra-queue-runner/builder.cc @@ -201,9 +201,11 @@ State::StepResult State::doBuildStep(nix::ref destStore, }; /* Do the build. */ + NarMemberDatas narMembers; + try { /* FIXME: referring builds may have conflicting timeouts. */ - buildRemote(destStore, machine, step, maxSilentTime, buildTimeout, repeats, result, activeStep, updateStep); + buildRemote(destStore, machine, step, maxSilentTime, buildTimeout, repeats, result, activeStep, updateStep, narMembers); } catch (Error & e) { if (activeStep->state_.lock()->cancelled) { printInfo("marking step %d of build %d as cancelled", stepNr, buildId); @@ -218,10 +220,8 @@ State::StepResult State::doBuildStep(nix::ref destStore, if (result.stepStatus == bsSuccess) { updateStep(ssPostProcessing); - res = getBuildOutput(destStore, ref(result.accessor), *step->drv); + res = getBuildOutput(destStore, narMembers, *step->drv); } - - result.accessor = 0; } time_t stepStopTime = time(0); diff --git a/src/hydra-queue-runner/nar-extractor.cc b/src/hydra-queue-runner/nar-extractor.cc new file mode 100644 index 00000000..b051cd19 --- /dev/null +++ b/src/hydra-queue-runner/nar-extractor.cc @@ -0,0 +1,82 @@ +#include "nar-extractor.hh" + +#include "archive.hh" + +#include + +using namespace nix; + +struct Extractor : ParseSink +{ + std::unordered_set filesToKeep { + "/nix-support/hydra-build-products", + "/nix-support/hydra-release-name", + "/nix-support/hydra-metrics", + }; + + NarMemberDatas & members; + NarMemberData * curMember = nullptr; + Path prefix; + + Extractor(NarMemberDatas & members, const Path & prefix) + : members(members), prefix(prefix) + { } + + void createDirectory(const Path & path) override + { + members.insert_or_assign(prefix + path, NarMemberData { .type = FSAccessor::Type::tDirectory }); + } + + void createRegularFile(const Path & path) override + { + curMember = &members.insert_or_assign(prefix + path, NarMemberData { + .type = FSAccessor::Type::tRegular, + .fileSize = 0, + .contents = filesToKeep.count(path) ? std::optional("") : std::nullopt, + }).first->second; + } + + std::optional expectedSize; + std::unique_ptr hashSink; + + void preallocateContents(unsigned long long size) override + { + expectedSize = size; + hashSink = std::make_unique(htSHA256); + } + + void receiveContents(unsigned char * data, unsigned int len) override + { + assert(expectedSize); + assert(curMember); + assert(hashSink); + *curMember->fileSize += len; + (*hashSink)(data, len); + if (curMember->contents) { + curMember->contents->append((char *) data, len); + } + assert(curMember->fileSize <= expectedSize); + if (curMember->fileSize == expectedSize) { + auto [hash, len] = hashSink->finish(); + assert(curMember->fileSize == len); + curMember->sha256 = hash; + hashSink.reset(); + } + } + + void createSymlink(const Path & path, const string & target) override + { + members.insert_or_assign(prefix + path, NarMemberData { .type = FSAccessor::Type::tSymlink }); + } +}; + + +void extractNarData( + Source & source, + const Path & prefix, + NarMemberDatas & members) +{ + Extractor extractor(members, prefix); + parseDump(extractor, source); + // Note: this point may not be reached if we're in a coroutine. +} diff --git a/src/hydra-queue-runner/nar-extractor.hh b/src/hydra-queue-runner/nar-extractor.hh new file mode 100644 index 00000000..d14726a3 --- /dev/null +++ b/src/hydra-queue-runner/nar-extractor.hh @@ -0,0 +1,23 @@ +#pragma once + +#include "fs-accessor.hh" +#include "types.hh" +#include "serialise.hh" +#include "hash.hh" + +struct NarMemberData +{ + nix::FSAccessor::Type type; + std::optional fileSize; + std::optional contents; + std::optional sha256; +}; + +typedef std::map NarMemberDatas; + +/* Read a NAR from a source and get to some info about every file + inside the NAR. */ +void extractNarData( + nix::Source & source, + const nix::Path & prefix, + NarMemberDatas & members); diff --git a/src/hydra-queue-runner/queue-monitor.cc b/src/hydra-queue-runner/queue-monitor.cc index 5bfcb1eb..0a900a87 100644 --- a/src/hydra-queue-runner/queue-monitor.cc +++ b/src/hydra-queue-runner/queue-monitor.cc @@ -672,5 +672,6 @@ BuildOutput State::getBuildOutputCached(Connection & conn, nix::ref } - return getBuildOutput(destStore, destStore->getFSAccessor(), drv); + NarMemberDatas narMembers; + return getBuildOutput(destStore, narMembers, drv); } diff --git a/src/hydra-queue-runner/state.hh b/src/hydra-queue-runner/state.hh index 28364db0..57fd5a77 100644 --- a/src/hydra-queue-runner/state.hh +++ b/src/hydra-queue-runner/state.hh @@ -14,6 +14,7 @@ #include "pool.hh" #include "store-api.hh" #include "sync.hh" +#include "nar-extractor.hh" typedef unsigned int BuildID; @@ -64,7 +65,6 @@ struct RemoteResult time_t startTime = 0, stopTime = 0; unsigned int overhead = 0; nix::Path logFile; - std::shared_ptr accessor; BuildStatus buildStatus() const { @@ -518,7 +518,8 @@ private: unsigned int maxSilentTime, unsigned int buildTimeout, unsigned int repeats, RemoteResult & result, std::shared_ptr activeStep, - std::function updateStep); + std::function updateStep, + NarMemberDatas & narMembers); void markSucceededBuild(pqxx::work & txn, Build::ptr build, const BuildOutput & res, bool isCachedBuild, time_t startTime, time_t stopTime); From a0e24f446baf27c593f3e04ecfed7da5b4b1d582 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 27 Jul 2020 20:40:57 +0200 Subject: [PATCH 7/9] Remove unused getMemSize() function --- src/hydra-queue-runner/hydra-queue-runner.cc | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/hydra-queue-runner/hydra-queue-runner.cc b/src/hydra-queue-runner/hydra-queue-runner.cc index 0da14bcd..0c7e26f5 100644 --- a/src/hydra-queue-runner/hydra-queue-runner.cc +++ b/src/hydra-queue-runner/hydra-queue-runner.cc @@ -30,13 +30,6 @@ template<> void toJSON(std::ostream & str, const double & n) { str << n; } -static uint64_t getMemSize() -{ - auto pages = sysconf(_SC_PHYS_PAGES); - return pages >= 0 ? pages * sysconf(_SC_PAGESIZE) : 4ULL << 30; -} - - std::string getEnvOrDie(const std::string & key) { auto value = getEnv(key); From 58a8b1c91cf8ed234a3e4ba61e45dd1f064fb945 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 28 Jul 2020 11:47:44 +0200 Subject: [PATCH 8/9] Keep the SHA-1 column in existing installations --- src/sql/upgrade-68.sql | 1 - 1 file changed, 1 deletion(-) delete mode 100644 src/sql/upgrade-68.sql diff --git a/src/sql/upgrade-68.sql b/src/sql/upgrade-68.sql deleted file mode 100644 index 368ed82b..00000000 --- a/src/sql/upgrade-68.sql +++ /dev/null @@ -1 +0,0 @@ -alter table BuildProducts drop column sha1hash; From 8722927c085e4e5a460d5d7cf73d8ac0161af4fd Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 28 Jul 2020 13:46:57 +0200 Subject: [PATCH 9/9] Copy paths in the right order --- src/hydra-queue-runner/build-remote.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index b7d5ff85..356e8dea 100644 --- a/src/hydra-queue-runner/build-remote.cc +++ b/src/hydra-queue-runner/build-remote.cc @@ -125,7 +125,7 @@ static void copyClosureTo(std::timed_mutex & sendMutex, ref destStore, // FIXME: use Store::topoSortPaths(). -StorePaths topoSortPaths(const std::map & paths) +StorePaths reverseTopoSortPaths(const std::map & paths) { StorePaths sorted; StorePathSet visited; @@ -150,8 +150,6 @@ StorePaths topoSortPaths(const std::map & paths) for (auto & i : paths) dfsVisit(i.first); - std::reverse(sorted.begin(), sorted.end()); - return sorted; } @@ -468,7 +466,7 @@ void State::buildRemote(ref destStore, printMsg(lvlDebug, "copying outputs of ‘%s’ from ‘%s’ (%d bytes)", localStore->printStorePath(step->drvPath), machine->sshName, totalNarSize); - auto pathsSorted = topoSortPaths(infos); + auto pathsSorted = reverseTopoSortPaths(infos); for (auto & path : pathsSorted) { auto & info = infos.find(path)->second;