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)
2020-10-20 15:14:02 +02:00 · 2020-10-20 15:14:02 +02:00 · 3ac9d74eb1
parent 58cdab64ac
commit 3ac9d74eb1
5 changed files with 172 additions and 80 deletions
--- 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<Derivation *>(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});
 }


--- a/src/libstore/ca-specific-schema.sql
+++ 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
+);
--- 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<void>([&]() {
+        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<void>([&]() {
        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<std::string, std::optional<StorePath>> LocalStore::queryPartialDerivationOutputMap(const StorePath & path_)
+// Try to resolve the derivation at path `original`, with a caching layer
+// to make it more efficient
+std::optional<StorePath> cachedResolve(
+    LocalStore & store,
+    const StorePath & original)
 {
-    auto path = path_;
-    std::map<std::string, std::optional<StorePath>> 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<std::map<std::string, std::optional<StorePath>>>([&]() {
-        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<std::string, std::optional<StorePath>>
+LocalStore::queryPartialDerivationOutputMap(const StorePath& path_)
+{
+    auto path = path_;
+    auto outputs = retrySQLite<std::map<std::string, std::optional<StorePath>>>([&]() {
+        auto state(_state.lock());
+        std::map<std::string, std::optional<StorePath>> 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<void>([&]() {
+        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<StorePath> LocalStore::queryPathFromHashPart(const std::string & hashPart)
 {
@ -1615,13 +1690,19 @@ void LocalStore::createUser(const std::string & userName, uid_t userId)
    }
 }

-std::optional<const DrvOutputInfo> 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<const Realisation> LocalStore::queryRealisation(
+    const DrvOutput& id) {
+    typedef std::optional<const Realisation> Ret;
+    return retrySQLite<Ret>([&]() -> 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
--- 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<const Realisation> queryRealisation(const DrvOutput&) override;

--- 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))