From 3ac9d74eb1de0f696bb0384132f7ecc7d057f9d6 Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 20 Oct 2020 15:14:02 +0200 Subject: [PATCH] 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))