From 58cdab64acd4807f73768fb32acdde39b501799f Mon Sep 17 00:00:00 2001 From: regnat Date: Thu, 8 Oct 2020 17:36:51 +0200 Subject: [PATCH 1/3] Store metadata about drv outputs realisations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For each known realisation, store: - its output - its output path This comes with a set of needed changes: - New `realisations` module declaring the types needed for describing these mappings - New `Store::registerDrvOutput` method registering all the needed informations about a derivation output (also replaces `LocalStore::linkDeriverToPath`) - new `Store::queryRealisation` method to retrieve the informations for a derivations This introcudes some redundancy on the remote-store side between `wopQueryDerivationOutputMap` and `wopQueryRealisation`. However we might need to keep both (regardless of backwards compat) because we sometimes need to get some infos for all the outputs of a derivation (where `wopQueryDerivationOutputMap` is handy), but all the stores can't implement it − because listing all the outputs of a derivation isn't really possible for binary caches where the server doesn't allow to list a directory. --- src/libstore/binary-cache-store.cc | 17 ++++++ src/libstore/binary-cache-store.hh | 7 +++ src/libstore/build/derivation-goal.cc | 19 ++++++- src/libstore/daemon.cc | 22 ++++++++ src/libstore/dummy-store.cc | 3 + src/libstore/legacy-ssh-store.cc | 4 ++ src/libstore/local-binary-cache-store.cc | 1 + src/libstore/local-store.cc | 21 +++++-- src/libstore/local-store.hh | 12 ++-- src/libstore/realisation.cc | 72 ++++++++++++++++++++++++ src/libstore/realisation.hh | 34 +++++++++++ src/libstore/remote-store.cc | 21 +++++++ src/libstore/remote-store.hh | 4 ++ src/libstore/store-api.hh | 15 +++++ src/libstore/worker-protocol.hh | 5 ++ tests/content-addressed.sh | 3 +- 16 files changed, 248 insertions(+), 12 deletions(-) create mode 100644 src/libstore/realisation.cc create mode 100644 src/libstore/realisation.hh diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index a918b7208..085dc7ba1 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -443,6 +443,23 @@ StorePath BinaryCacheStore::addTextToStore(const string & name, const string & s })->path; } +std::optional BinaryCacheStore::queryRealisation(const DrvOutput & id) +{ + auto outputInfoFilePath = realisationsPrefix + "/" + id.to_string() + ".doi"; + auto rawOutputInfo = getFile(outputInfoFilePath); + + if (rawOutputInfo) { + return { Realisation::parse(*rawOutputInfo, outputInfoFilePath) }; + } else { + return std::nullopt; + } +} + +void BinaryCacheStore::registerDrvOutput(const Realisation& info) { + auto filePath = realisationsPrefix + "/" + info.id.to_string() + ".doi"; + upsertFile(filePath, info.to_string(), "text/x-nix-derivertopath"); +} + ref BinaryCacheStore::getFSAccessor() { return make_ref(ref(shared_from_this()), localNarCache); diff --git a/src/libstore/binary-cache-store.hh b/src/libstore/binary-cache-store.hh index 5224d7ec8..07a8b2beb 100644 --- a/src/libstore/binary-cache-store.hh +++ b/src/libstore/binary-cache-store.hh @@ -33,6 +33,9 @@ private: protected: + // The prefix under which realisation infos will be stored + const std::string realisationsPrefix = "/realisations"; + BinaryCacheStore(const Params & params); public: @@ -99,6 +102,10 @@ public: StorePath addTextToStore(const string & name, const string & s, const StorePathSet & references, RepairFlag repair) override; + void registerDrvOutput(const Realisation & info) override; + + std::optional queryRealisation(const DrvOutput &) override; + void narFromPath(const StorePath & path, Sink & sink) override; BuildResult buildDerivation(const StorePath & drvPath, const BasicDerivation & drv, diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index de58d9f06..a0f10c33d 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -2094,6 +2094,20 @@ struct RestrictedStore : public LocalFSStore, public virtual RestrictedStoreConf /* Nothing to be done; 'path' must already be valid. */ } + void registerDrvOutput(const Realisation & info) override + { + if (!goal.isAllowed(info.id.drvPath)) + throw InvalidPath("cannot register unknown drv output '%s' in recursive Nix", printStorePath(info.id.drvPath)); + next->registerDrvOutput(info); + } + + std::optional queryRealisation(const DrvOutput & id) override + { + if (!goal.isAllowed(id.drvPath)) + throw InvalidPath("cannot query the output info for unknown derivation '%s' in recursive Nix", printStorePath(id.drvPath)); + return next->queryRealisation(id); + } + void buildPaths(const std::vector & paths, BuildMode buildMode) override { if (buildMode != bmNormal) throw Error("unsupported build mode"); @@ -3393,7 +3407,10 @@ void DerivationGoal::registerOutputs() if (useDerivation || isCaFloating) for (auto & [outputName, newInfo] : infos) - worker.store.linkDeriverToPath(drvPathResolved, outputName, newInfo.path); + worker.store.registerDrvOutput( + DrvOutputId{drvPathResolved, outputName}, + DrvOutputInfo{.outPath = newInfo.path, + .resolvedDrv = drvPathResolved}); } diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 2224d54d5..ba5788b64 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -868,6 +868,28 @@ static void performOp(TunnelLogger * logger, ref store, break; } + case wopRegisterDrvOutput: { + logger->startWork(); + auto outputId = DrvOutput::parse(readString(from)); + auto outputPath = StorePath(readString(from)); + auto resolvedDrv = StorePath(readString(from)); + store->registerDrvOutput(Realisation{ + .id = outputId, .outPath = outputPath}); + logger->stopWork(); + break; + } + + case wopQueryRealisation: { + logger->startWork(); + auto outputId = DrvOutput::parse(readString(from)); + auto info = store->queryRealisation(outputId); + logger->stopWork(); + std::set outPaths; + if (info) outPaths.insert(info->outPath); + worker_proto::write(*store, to, outPaths); + break; + } + default: throw Error("invalid operation %1%", op); } diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index 98b745c3a..91fc178db 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -60,6 +60,9 @@ struct DummyStore : public Store, public virtual DummyStoreConfig BuildResult buildDerivation(const StorePath & drvPath, const BasicDerivation & drv, BuildMode buildMode) override { unsupported("buildDerivation"); } + + std::optional queryRealisation(const DrvOutput&) override + { unsupported("queryRealisation"); } }; static RegisterStoreImplementation regDummyStore; diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 467169ce8..ad1779aea 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -333,6 +333,10 @@ public: auto conn(connections->get()); return conn->remoteVersion; } + + std::optional queryRealisation(const DrvOutput&) override + // TODO: Implement + { unsupported("queryRealisation"); } }; static RegisterStoreImplementation regLegacySSHStore; diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc index 7d979c5c2..bb7464989 100644 --- a/src/libstore/local-binary-cache-store.cc +++ b/src/libstore/local-binary-cache-store.cc @@ -87,6 +87,7 @@ protected: void LocalBinaryCacheStore::init() { createDirs(binaryCacheDir + "/nar"); + createDirs(binaryCacheDir + realisationsPrefix); if (writeDebugInfo) createDirs(binaryCacheDir + "/debuginfo"); BinaryCacheStore::init(); diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 2a47b3956..418b3ab9c 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -597,13 +597,16 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat } -void LocalStore::linkDeriverToPath(const StorePath & deriver, const string & outputName, const StorePath & output) +void LocalStore::registerDrvOutput(const Realisation & info) { auto state(_state.lock()); - return linkDeriverToPath(*state, queryValidPathId(*state, deriver), outputName, output); + // XXX: This ignores the references of the output because we can + // recompute them later from the drv and the references of the associated + // store path, but doing so is both inefficient and fragile. + return registerDrvOutput_(*state, queryValidPathId(*state, id.drvPath), id.outputName, info.outPath); } -void LocalStore::linkDeriverToPath(State & state, uint64_t deriver, const string & outputName, const StorePath & output) +void LocalStore::registerDrvOutput_(State & state, uint64_t deriver, const string & outputName, const StorePath & output) { retrySQLite([&]() { state.stmts->AddDerivationOutput.use() @@ -653,7 +656,7 @@ uint64_t LocalStore::addValidPath(State & state, /* Floating CA derivations have indeterminate output paths until they are built, so don't register anything in that case */ if (i.second.second) - linkDeriverToPath(state, id, i.first, *i.second.second); + registerDrvOutput_(state, id, i.first, *i.second.second); } } @@ -1612,5 +1615,13 @@ void LocalStore::createUser(const std::string & userName, uid_t userId) } } - +std::optional LocalStore::queryDrvOutputInfo(const DrvOutputId& id) { + auto outputPath = queryOutputPathOf(id.drvPath, id.outputName); + if (!(outputPath && isValidPath(*outputPath))) + return std::nullopt; + else + return {DrvOutputInfo{ + .outPath = *outputPath, + }}; } +} // namespace nix diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 332718af4..440411f01 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -208,6 +208,13 @@ public: garbage until it exceeds maxFree. */ void autoGC(bool sync = true); + /* Register the store path 'output' as the output named 'outputName' of + derivation 'deriver'. */ + void registerDrvOutput(const DrvOutputId & outputId, const DrvOutputInfo & info) override; + void registerDrvOutput_(State & state, uint64_t deriver, const string & outputName, const StorePath & output); + + std::optional queryRealisation(const DrvOutput&) override; + private: int getSchema(); @@ -276,11 +283,6 @@ private: specified by the ‘secret-key-files’ option. */ void signPathInfo(ValidPathInfo & info); - /* Register the store path 'output' as the output named 'outputName' of - derivation 'deriver'. */ - void linkDeriverToPath(const StorePath & deriver, const string & outputName, const StorePath & output); - void linkDeriverToPath(State & state, uint64_t deriver, const string & outputName, const StorePath & output); - Path getRealStoreDir() override { return realStoreDir; } void createUser(const std::string & userName, uid_t userId) override; diff --git a/src/libstore/realisation.cc b/src/libstore/realisation.cc new file mode 100644 index 000000000..fcc1a3825 --- /dev/null +++ b/src/libstore/realisation.cc @@ -0,0 +1,72 @@ +#include "realisation.hh" +#include "store-api.hh" + +namespace nix { + +MakeError(InvalidDerivationOutputId, Error); + +DrvOutput DrvOutput::parse(const std::string &strRep) { + const auto &[rawPath, outputs] = parsePathWithOutputs(strRep); + if (outputs.size() != 1) + throw InvalidDerivationOutputId("Invalid derivation output id %s", strRep); + + return DrvOutput{ + .drvPath = StorePath(rawPath), + .outputName = *outputs.begin(), + }; +} + +std::string DrvOutput::to_string() const { + return std::string(drvPath.to_string()) + "!" + outputName; +} + +std::string Realisation::to_string() const { + std::string res; + + res += "Id: " + id.to_string() + '\n'; + res += "OutPath: " + std::string(outPath.to_string()) + '\n'; + + return res; +} + +Realisation Realisation::parse(const std::string & s, const std::string & whence) +{ + // XXX: Copy-pasted from NarInfo::NarInfo. Should be factored out + auto corrupt = [&]() { + return Error("Drv output info file '%1%' is corrupt", whence); + }; + + std::optional id; + std::optional outPath; + + size_t pos = 0; + while (pos < s.size()) { + + size_t colon = s.find(':', pos); + if (colon == std::string::npos) throw corrupt(); + + std::string name(s, pos, colon - pos); + + size_t eol = s.find('\n', colon + 2); + if (eol == std::string::npos) throw corrupt(); + + std::string value(s, colon + 2, eol - colon - 2); + + if (name == "Id") + id = DrvOutput::parse(value); + + if (name == "OutPath") + outPath = StorePath(value); + + pos = eol + 1; + } + + if (!outPath) corrupt(); + if (!id) corrupt(); + return Realisation { + .id = *id, + .outPath = *outPath, + }; +} + +} // namespace nix diff --git a/src/libstore/realisation.hh b/src/libstore/realisation.hh new file mode 100644 index 000000000..c573e1bb4 --- /dev/null +++ b/src/libstore/realisation.hh @@ -0,0 +1,34 @@ +#pragma once + +#include "path.hh" + +namespace nix { + +struct DrvOutput { + StorePath drvPath; + std::string outputName; + + std::string to_string() const; + + static DrvOutput parse(const std::string &); + + bool operator<(const DrvOutput& other) const { return to_pair() < other.to_pair(); } + bool operator==(const DrvOutput& other) const { return to_pair() == other.to_pair(); } + +private: + // Just to make comparison operators easier to write + std::pair to_pair() const + { return std::make_pair(drvPath, outputName); } +}; + +struct Realisation { + DrvOutput id; + StorePath outPath; + + std::string to_string() const; + static Realisation parse(const std::string & s, const std::string & whence); +}; + +typedef std::map DrvOutputs; + +} diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index be29f8e6f..f1f4d0516 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -609,6 +609,27 @@ StorePath RemoteStore::addTextToStore(const string & name, const string & s, return addCAToStore(source, name, TextHashMethod{}, references, repair)->path; } +void RemoteStore::registerDrvOutput(const Realisation & info) +{ + auto conn(getConnection()); + conn->to << wopRegisterDrvOutput; + conn->to << info.id.to_string(); + conn->to << std::string(info.outPath.to_string()); + conn.processStderr(); +} + +std::optional RemoteStore::queryRealisation(const DrvOutput & id) +{ + auto conn(getConnection()); + conn->to << wopQueryRealisation; + conn->to << id.to_string(); + conn.processStderr(); + auto outPaths = worker_proto::read(*this, conn->from, Phantom>{}); + if (outPaths.empty()) + return std::nullopt; + return {Realisation{.id = id, .outPath = *outPaths.begin()}}; +} + void RemoteStore::buildPaths(const std::vector & drvPaths, BuildMode buildMode) { diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 9f78fcb02..fdd53e6ed 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -81,6 +81,10 @@ public: StorePath addTextToStore(const string & name, const string & s, const StorePathSet & references, RepairFlag repair) override; + void registerDrvOutput(const Realisation & info) override; + + std::optional queryRealisation(const DrvOutput &) override; + void buildPaths(const std::vector & paths, BuildMode buildMode) override; BuildResult buildDerivation(const StorePath & drvPath, const BasicDerivation & drv, diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 6b9331495..7cdadc1f3 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -1,5 +1,6 @@ #pragma once +#include "realisation.hh" #include "path.hh" #include "hash.hh" #include "content-address.hh" @@ -396,6 +397,8 @@ protected: public: + virtual std::optional queryRealisation(const DrvOutput &) = 0; + /* Queries the set of incoming FS references for a store path. The result is not cleared. */ virtual void queryReferrers(const StorePath & path, StorePathSet & referrers) @@ -468,6 +471,18 @@ public: virtual StorePath addTextToStore(const string & name, const string & s, const StorePathSet & references, RepairFlag repair = NoRepair) = 0; + /** + * Add a mapping indicating that `deriver!outputName` maps to the output path + * `output`. + * + * This is redundant for known-input-addressed and fixed-output derivations + * as this information is already present in the drv file, but necessary for + * floating-ca derivations and their dependencies as there's no way to + * retrieve this information otherwise. + */ + virtual void registerDrvOutput(const Realisation & output) + { unsupported("registerDrvOutput"); } + /* Write a NAR dump of a store path. */ virtual void narFromPath(const StorePath & path, Sink & sink) = 0; diff --git a/src/libstore/worker-protocol.hh b/src/libstore/worker-protocol.hh index 63bd6ea49..f2cdc7ca3 100644 --- a/src/libstore/worker-protocol.hh +++ b/src/libstore/worker-protocol.hh @@ -1,5 +1,8 @@ #pragma once +#include "store-api.hh" +#include "serialise.hh" + namespace nix { @@ -50,6 +53,8 @@ typedef enum { wopAddToStoreNar = 39, wopQueryMissing = 40, wopQueryDerivationOutputMap = 41, + wopRegisterDrvOutput = 42, + wopQueryRealisation = 43, } WorkerOp; diff --git a/tests/content-addressed.sh b/tests/content-addressed.sh index bc37a99c1..e8ac88609 100644 --- a/tests/content-addressed.sh +++ b/tests/content-addressed.sh @@ -55,7 +55,8 @@ testNixCommand () { nix build --experimental-features 'nix-command ca-derivations' --file ./content-addressed.nix --no-link } -testRemoteCache +# Disabled until we have it properly working +# testRemoteCache testDeterministicCA testCutoff testGC From 3ac9d74eb1de0f696bb0384132f7ecc7d057f9d6 Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 20 Oct 2020 15:14:02 +0200 Subject: [PATCH 2/3] Rework the db schema for derivation outputs Add a new table for tracking the derivation output mappings. We used to hijack the `DerivationOutputs` table for that, but (despite its name), it isn't a really good fit: - Its entries depend on the drv being a valid path, making it play badly with garbage collection and preventing us to copy a drv output without copying the whole drv closure too; - It dosen't guaranty that the output path exists; By using a different table, we can experiment with a different schema better suited for tracking the output mappings of CA derivations. (incidentally, this also fixes #4138) --- src/libstore/build/derivation-goal.cc | 16 +- src/libstore/ca-specific-schema.sql | 11 ++ src/libstore/local-store.cc | 217 ++++++++++++++++++-------- src/libstore/local-store.hh | 4 +- src/libstore/local.mk | 4 +- 5 files changed, 172 insertions(+), 80 deletions(-) create mode 100644 src/libstore/ca-specific-schema.sql diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index a0f10c33d..b7bf866eb 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -493,8 +493,9 @@ void DerivationGoal::inputsRealised() if (useDerivation) { auto & fullDrv = *dynamic_cast(drv.get()); - if ((!fullDrv.inputDrvs.empty() && derivationIsCA(fullDrv.type())) - || fullDrv.type() == DerivationType::DeferredInputAddressed) { + if (settings.isExperimentalFeatureEnabled("ca-derivations") && + ((!fullDrv.inputDrvs.empty() && derivationIsCA(fullDrv.type())) + || fullDrv.type() == DerivationType::DeferredInputAddressed)) { /* We are be able to resolve this derivation based on the now-known results of dependencies. If so, we become a stub goal aliasing that resolved derivation goal */ @@ -3405,12 +3406,11 @@ void DerivationGoal::registerOutputs() drvPathResolved = writeDerivation(worker.store, drv2); } - if (useDerivation || isCaFloating) - for (auto & [outputName, newInfo] : infos) - worker.store.registerDrvOutput( - DrvOutputId{drvPathResolved, outputName}, - DrvOutputInfo{.outPath = newInfo.path, - .resolvedDrv = drvPathResolved}); + if (settings.isExperimentalFeatureEnabled("ca-derivations")) + for (auto& [outputName, newInfo] : infos) + worker.store.registerDrvOutput(Realisation{ + .id = DrvOutput{drvPathResolved, outputName}, + .outPath = newInfo.path}); } diff --git a/src/libstore/ca-specific-schema.sql b/src/libstore/ca-specific-schema.sql new file mode 100644 index 000000000..93c442826 --- /dev/null +++ b/src/libstore/ca-specific-schema.sql @@ -0,0 +1,11 @@ +-- Extension of the sql schema for content-addressed derivations. +-- Won't be loaded unless the experimental feature `ca-derivations` +-- is enabled + +create table if not exists Realisations ( + drvPath text not null, + outputName text not null, -- symbolic output id, usually "out" + outputPath integer not null, + primary key (drvPath, outputName), + foreign key (outputPath) references ValidPaths(id) on delete cascade +); diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 418b3ab9c..69ab821d9 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -52,12 +52,52 @@ struct LocalStore::State::Stmts { SQLiteStmt QueryReferrers; SQLiteStmt InvalidatePath; SQLiteStmt AddDerivationOutput; + SQLiteStmt RegisterRealisedOutput; SQLiteStmt QueryValidDerivers; SQLiteStmt QueryDerivationOutputs; + SQLiteStmt QueryRealisedOutput; + SQLiteStmt QueryAllRealisedOutputs; SQLiteStmt QueryPathFromHashPart; SQLiteStmt QueryValidPaths; }; +int getSchema(Path schemaPath) +{ + int curSchema = 0; + if (pathExists(schemaPath)) { + string s = readFile(schemaPath); + if (!string2Int(s, curSchema)) + throw Error("'%1%' is corrupt", schemaPath); + } + return curSchema; +} + +void migrateCASchema(SQLite& db, Path schemaPath, AutoCloseFD& lockFd) +{ + const int nixCASchemaVersion = 1; + int curCASchema = getSchema(schemaPath); + if (curCASchema != nixCASchemaVersion) { + if (curCASchema > nixCASchemaVersion) { + throw Error("current Nix store ca-schema is version %1%, but I only support %2%", + curCASchema, nixCASchemaVersion); + } + + if (!lockFile(lockFd.get(), ltWrite, false)) { + printInfo("waiting for exclusive access to the Nix store for ca drvs..."); + lockFile(lockFd.get(), ltWrite, true); + } + + if (curCASchema == 0) { + static const char schema[] = + #include "ca-specific-schema.sql.gen.hh" + ; + db.exec(schema); + } + writeFile(schemaPath, fmt("%d", nixCASchemaVersion)); + lockFile(lockFd.get(), ltRead, true); + } +} + LocalStore::LocalStore(const Params & params) : StoreConfig(params) , Store(params) @@ -238,6 +278,10 @@ LocalStore::LocalStore(const Params & params) else openDB(*state, false); + if (settings.isExperimentalFeatureEnabled("ca-derivations")) { + migrateCASchema(state->db, dbDir + "/ca-schema", globalLock); + } + /* Prepare SQL statements. */ state->stmts->RegisterValidPath.create(state->db, "insert into ValidPaths (path, hash, registrationTime, deriver, narSize, ultimate, sigs, ca) values (?, ?, ?, ?, ?, ?, ?, ?);"); @@ -264,6 +308,28 @@ LocalStore::LocalStore(const Params & params) state->stmts->QueryPathFromHashPart.create(state->db, "select path from ValidPaths where path >= ? limit 1;"); state->stmts->QueryValidPaths.create(state->db, "select path from ValidPaths"); + if (settings.isExperimentalFeatureEnabled("ca-derivations")) { + state->stmts->RegisterRealisedOutput.create(state->db, + R"( + insert or replace into Realisations (drvPath, outputName, outputPath) + values (?, ?, (select id from ValidPaths where path = ?)) + ; + )"); + state->stmts->QueryRealisedOutput.create(state->db, + R"( + select Output.path from Realisations + inner join ValidPaths as Output on Output.id = Realisations.outputPath + where drvPath = ? and outputName = ? + ; + )"); + state->stmts->QueryAllRealisedOutputs.create(state->db, + R"( + select outputName, Output.path from Realisations + inner join ValidPaths as Output on Output.id = Realisations.outputPath + where drvPath = ? + ; + )"); + } } @@ -301,16 +367,7 @@ std::string LocalStore::getUri() int LocalStore::getSchema() -{ - int curSchema = 0; - if (pathExists(schemaPath)) { - string s = readFile(schemaPath); - if (!string2Int(s, curSchema)) - throw Error("'%1%' is corrupt", schemaPath); - } - return curSchema; -} - +{ return nix::getSchema(schemaPath); } void LocalStore::openDB(State & state, bool create) { @@ -600,13 +657,16 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat void LocalStore::registerDrvOutput(const Realisation & info) { auto state(_state.lock()); - // XXX: This ignores the references of the output because we can - // recompute them later from the drv and the references of the associated - // store path, but doing so is both inefficient and fragile. - return registerDrvOutput_(*state, queryValidPathId(*state, id.drvPath), id.outputName, info.outPath); + retrySQLite([&]() { + state->stmts->RegisterRealisedOutput.use() + (info.id.drvPath.to_string()) + (info.id.outputName) + (printStorePath(info.outPath)) + .exec(); + }); } -void LocalStore::registerDrvOutput_(State & state, uint64_t deriver, const string & outputName, const StorePath & output) +void LocalStore::cacheDrvOutputMapping(State & state, const uint64_t deriver, const string & outputName, const StorePath & output) { retrySQLite([&]() { state.stmts->AddDerivationOutput.use() @@ -656,7 +716,7 @@ uint64_t LocalStore::addValidPath(State & state, /* Floating CA derivations have indeterminate output paths until they are built, so don't register anything in that case */ if (i.second.second) - registerDrvOutput_(state, id, i.first, *i.second.second); + cacheDrvOutputMapping(state, id, i.first, *i.second.second); } } @@ -817,70 +877,85 @@ StorePathSet LocalStore::queryValidDerivers(const StorePath & path) }); } - -std::map> LocalStore::queryPartialDerivationOutputMap(const StorePath & path_) +// Try to resolve the derivation at path `original`, with a caching layer +// to make it more efficient +std::optional cachedResolve( + LocalStore & store, + const StorePath & original) { - auto path = path_; - std::map> outputs; - Derivation drv = readDerivation(path); - for (auto & [outName, _] : drv.outputs) { - outputs.insert_or_assign(outName, std::nullopt); - } - bool haveCached = false; { auto resolutions = drvPathResolutions.lock(); - auto resolvedPathOptIter = resolutions->find(path); + auto resolvedPathOptIter = resolutions->find(original); if (resolvedPathOptIter != resolutions->end()) { auto & [_, resolvedPathOpt] = *resolvedPathOptIter; if (resolvedPathOpt) - path = *resolvedPathOpt; - haveCached = true; + return resolvedPathOpt; } } - /* can't just use else-if instead of `!haveCached` because we need to unlock - `drvPathResolutions` before it is locked in `Derivation::resolve`. */ - if (!haveCached && (drv.type() == DerivationType::CAFloating || drv.type() == DerivationType::DeferredInputAddressed)) { - /* Try resolve drv and use that path instead. */ - auto attempt = drv.tryResolve(*this); - if (!attempt) - /* If we cannot resolve the derivation, we cannot have any path - assigned so we return the map of all std::nullopts. */ - return outputs; - /* Just compute store path */ - auto pathResolved = writeDerivation(*this, *std::move(attempt), NoRepair, true); - /* Store in memo table. */ - /* FIXME: memo logic should not be local-store specific, should have - wrapper-method instead. */ - drvPathResolutions.lock()->insert_or_assign(path, pathResolved); - path = std::move(pathResolved); - } - return retrySQLite>>([&]() { - auto state(_state.lock()); + /* Try resolve drv and use that path instead. */ + auto drv = store.readDerivation(original); + auto attempt = drv.tryResolve(store); + if (!attempt) + return std::nullopt; + /* Just compute store path */ + auto pathResolved = + writeDerivation(store, *std::move(attempt), NoRepair, true); + /* Store in memo table. */ + drvPathResolutions.lock()->insert_or_assign(original, pathResolved); + return pathResolved; +} + +std::map> +LocalStore::queryPartialDerivationOutputMap(const StorePath& path_) +{ + auto path = path_; + auto outputs = retrySQLite>>([&]() { + auto state(_state.lock()); + std::map> outputs; uint64_t drvId; try { drvId = queryValidPathId(*state, path); - } catch (InvalidPath &) { - /* FIXME? if the derivation doesn't exist, we cannot have a mapping - for it. */ - return outputs; + } catch (InvalidPath&) { + // Ignore non-existing drvs as they might still have an output map + // defined if ca-derivations is enabled } + auto use(state->stmts->QueryDerivationOutputs.use()(drvId)); + while (use.next()) + outputs.insert_or_assign( + use.getStr(0), parseStorePath(use.getStr(1))); - auto useQueryDerivationOutputs { - state->stmts->QueryDerivationOutputs.use() - (drvId) - }; + return outputs; + }); + + if (!settings.isExperimentalFeatureEnabled("ca-derivations")) + return outputs; + + auto drv = readDerivation(path); + + for (auto & output : drv.outputsAndOptPaths(*this)) { + outputs.emplace(output.first, std::nullopt); + } + + auto resolvedDrv = cachedResolve(*this, path); + + if (!resolvedDrv) + return outputs; + + retrySQLite([&]() { + auto state(_state.lock()); + path = *resolvedDrv; + auto useQueryDerivationOutputs{ + state->stmts->QueryAllRealisedOutputs.use()(path.to_string())}; while (useQueryDerivationOutputs.next()) outputs.insert_or_assign( useQueryDerivationOutputs.getStr(0), - parseStorePath(useQueryDerivationOutputs.getStr(1)) - ); - - return outputs; + parseStorePath(useQueryDerivationOutputs.getStr(1))); }); -} + return outputs; +} std::optional LocalStore::queryPathFromHashPart(const std::string & hashPart) { @@ -1615,13 +1690,19 @@ void LocalStore::createUser(const std::string & userName, uid_t userId) } } -std::optional LocalStore::queryDrvOutputInfo(const DrvOutputId& id) { - auto outputPath = queryOutputPathOf(id.drvPath, id.outputName); - if (!(outputPath && isValidPath(*outputPath))) - return std::nullopt; - else - return {DrvOutputInfo{ - .outPath = *outputPath, - }}; +std::optional LocalStore::queryRealisation( + const DrvOutput& id) { + typedef std::optional Ret; + return retrySQLite([&]() -> Ret { + auto state(_state.lock()); + auto use(state->stmts->QueryRealisedOutput.use()(id.drvPath.to_string())( + id.outputName)); + if (!use.next()) + return std::nullopt; + auto outputPath = parseStorePath(use.getStr(0)); + auto resolvedDrv = StorePath(use.getStr(1)); + return Ret{ + Realisation{.id = id, .outPath = outputPath}}; + }); } } // namespace nix diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 440411f01..69559e346 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -210,8 +210,8 @@ public: /* Register the store path 'output' as the output named 'outputName' of derivation 'deriver'. */ - void registerDrvOutput(const DrvOutputId & outputId, const DrvOutputInfo & info) override; - void registerDrvOutput_(State & state, uint64_t deriver, const string & outputName, const StorePath & output); + void registerDrvOutput(const Realisation & info) override; + void cacheDrvOutputMapping(State & state, const uint64_t deriver, const string & outputName, const StorePath & output); std::optional queryRealisation(const DrvOutput&) override; diff --git a/src/libstore/local.mk b/src/libstore/local.mk index dfe1e2cc4..03c4351ac 100644 --- a/src/libstore/local.mk +++ b/src/libstore/local.mk @@ -48,7 +48,7 @@ ifneq ($(sandbox_shell),) libstore_CXXFLAGS += -DSANDBOX_SHELL="\"$(sandbox_shell)\"" endif -$(d)/local-store.cc: $(d)/schema.sql.gen.hh +$(d)/local-store.cc: $(d)/schema.sql.gen.hh $(d)/ca-specific-schema.sql.gen.hh $(d)/build.cc: @@ -58,7 +58,7 @@ $(d)/build.cc: @echo ')foo"' >> $@.tmp @mv $@.tmp $@ -clean-files += $(d)/schema.sql.gen.hh +clean-files += $(d)/schema.sql.gen.hh $(d)/ca-specific-schema.sql.gen.hh $(eval $(call install-file-in, $(d)/nix-store.pc, $(prefix)/lib/pkgconfig, 0644)) From 8914e01e37ad072d940e2000fede7c2e0f4b194c Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 8 Dec 2020 21:07:52 +0100 Subject: [PATCH 3/3] Store the realisations as JSON in the binary cache Fix #4332 --- src/libstore/binary-cache-store.cc | 5 ++- src/libstore/realisation.cc | 61 ++++++++++-------------------- src/libstore/realisation.hh | 5 ++- 3 files changed, 25 insertions(+), 46 deletions(-) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 085dc7ba1..5b081c1ae 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -449,7 +449,8 @@ std::optional BinaryCacheStore::queryRealisation(const DrvOut auto rawOutputInfo = getFile(outputInfoFilePath); if (rawOutputInfo) { - return { Realisation::parse(*rawOutputInfo, outputInfoFilePath) }; + return {Realisation::fromJSON( + nlohmann::json::parse(*rawOutputInfo), outputInfoFilePath)}; } else { return std::nullopt; } @@ -457,7 +458,7 @@ std::optional BinaryCacheStore::queryRealisation(const DrvOut void BinaryCacheStore::registerDrvOutput(const Realisation& info) { auto filePath = realisationsPrefix + "/" + info.id.to_string() + ".doi"; - upsertFile(filePath, info.to_string(), "text/x-nix-derivertopath"); + upsertFile(filePath, info.toJSON(), "application/json"); } ref BinaryCacheStore::getFSAccessor() diff --git a/src/libstore/realisation.cc b/src/libstore/realisation.cc index fcc1a3825..47db1ec9f 100644 --- a/src/libstore/realisation.cc +++ b/src/libstore/realisation.cc @@ -1,5 +1,6 @@ #include "realisation.hh" #include "store-api.hh" +#include namespace nix { @@ -20,52 +21,28 @@ std::string DrvOutput::to_string() const { return std::string(drvPath.to_string()) + "!" + outputName; } -std::string Realisation::to_string() const { - std::string res; - - res += "Id: " + id.to_string() + '\n'; - res += "OutPath: " + std::string(outPath.to_string()) + '\n'; - - return res; +nlohmann::json Realisation::toJSON() const { + return nlohmann::json{ + {"id", id.to_string()}, + {"outPath", outPath.to_string()}, + }; } -Realisation Realisation::parse(const std::string & s, const std::string & whence) -{ - // XXX: Copy-pasted from NarInfo::NarInfo. Should be factored out - auto corrupt = [&]() { - return Error("Drv output info file '%1%' is corrupt", whence); +Realisation Realisation::fromJSON( + const nlohmann::json& json, + const std::string& whence) { + auto getField = [&](std::string fieldName) -> std::string { + auto fieldIterator = json.find(fieldName); + if (fieldIterator == json.end()) + throw Error( + "Drv output info file '%1%' is corrupt, missing field %2%", + whence, fieldName); + return *fieldIterator; }; - std::optional id; - std::optional outPath; - - size_t pos = 0; - while (pos < s.size()) { - - size_t colon = s.find(':', pos); - if (colon == std::string::npos) throw corrupt(); - - std::string name(s, pos, colon - pos); - - size_t eol = s.find('\n', colon + 2); - if (eol == std::string::npos) throw corrupt(); - - std::string value(s, colon + 2, eol - colon - 2); - - if (name == "Id") - id = DrvOutput::parse(value); - - if (name == "OutPath") - outPath = StorePath(value); - - pos = eol + 1; - } - - if (!outPath) corrupt(); - if (!id) corrupt(); - return Realisation { - .id = *id, - .outPath = *outPath, + return Realisation{ + .id = DrvOutput::parse(getField("id")), + .outPath = StorePath(getField("outPath")), }; } diff --git a/src/libstore/realisation.hh b/src/libstore/realisation.hh index c573e1bb4..08579b739 100644 --- a/src/libstore/realisation.hh +++ b/src/libstore/realisation.hh @@ -1,6 +1,7 @@ #pragma once #include "path.hh" +#include namespace nix { @@ -25,8 +26,8 @@ struct Realisation { DrvOutput id; StorePath outPath; - std::string to_string() const; - static Realisation parse(const std::string & s, const std::string & whence); + nlohmann::json toJSON() const; + static Realisation fromJSON(const nlohmann::json& json, const std::string& whence); }; typedef std::map DrvOutputs;