Merge pull request #8243 from obsidiansystems/indirect-root-store

Refactor `Store` hierarchy with a new `IndirectRootStore` interface
This commit is contained in:
John Ericson 2023-07-24 10:03:34 -04:00 committed by GitHub
commit 40c77f3514
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 186 additions and 82 deletions

View file

@ -1,5 +1,5 @@
#include "local-derivation-goal.hh" #include "local-derivation-goal.hh"
#include "gc-store.hh" #include "indirect-root-store.hh"
#include "hook-instance.hh" #include "hook-instance.hh"
#include "worker.hh" #include "worker.hh"
#include "builtins.hh" #include "builtins.hh"
@ -1200,7 +1200,7 @@ struct RestrictedStoreConfig : virtual LocalFSStoreConfig
/* A wrapper around LocalStore that only allows building/querying of /* A wrapper around LocalStore that only allows building/querying of
paths that are in the input closures of the build or were added via paths that are in the input closures of the build or were added via
recursive Nix calls. */ recursive Nix calls. */
struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual LocalFSStore, public virtual GcStore struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual IndirectRootStore, public virtual GcStore
{ {
ref<LocalStore> next; ref<LocalStore> next;

View file

@ -7,6 +7,7 @@
#include "store-cast.hh" #include "store-cast.hh"
#include "gc-store.hh" #include "gc-store.hh"
#include "log-store.hh" #include "log-store.hh"
#include "indirect-root-store.hh"
#include "path-with-outputs.hh" #include "path-with-outputs.hh"
#include "finally.hh" #include "finally.hh"
#include "archive.hh" #include "archive.hh"
@ -675,8 +676,8 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
Path path = absPath(readString(from)); Path path = absPath(readString(from));
logger->startWork(); logger->startWork();
auto & gcStore = require<GcStore>(*store); auto & indirectRootStore = require<IndirectRootStore>(*store);
gcStore.addIndirectRoot(path); indirectRootStore.addIndirectRoot(path);
logger->stopWork(); logger->stopWork();
to << 1; to << 1;

View file

@ -71,19 +71,36 @@ struct GCResults
}; };
/**
* Mix-in class for \ref Store "stores" which expose a notion of garbage
* collection.
*
* Garbage collection will allow deleting paths which are not
* transitively "rooted".
*
* The notion of GC roots actually not part of this class.
*
* - The base `Store` class has `Store::addTempRoot()` because for a store
* that doesn't support garbage collection at all, a temporary GC root is
* safely implementable as no-op.
*
* @todo actually this is not so good because stores are *views*.
* Some views have only a no-op temp roots even though others to the
* same store allow triggering GC. For instance one can't add a root
* over ssh, but that doesn't prevent someone from gc-ing that store
* accesed via SSH locally).
*
* - The derived `LocalFSStore` class has `LocalFSStore::addPermRoot`,
* which is not part of this class because it relies on the notion of
* an ambient file system. There are stores (`ssh-ng://`, for one),
* that *do* support garbage collection but *don't* expose any file
* system, and `LocalFSStore::addPermRoot` thus does not make sense
* for them.
*/
struct GcStore : public virtual Store struct GcStore : public virtual Store
{ {
inline static std::string operationName = "Garbage collection"; inline static std::string operationName = "Garbage collection";
/**
* Add an indirect root, which is merely a symlink to `path` from
* `/nix/var/nix/gcroots/auto/<hash of path>`. `path` is supposed
* to be a symlink to a store path. The garbage collector will
* automatically remove the indirect root when it finds that
* `path` has disappeared.
*/
virtual void addIndirectRoot(const Path & path) = 0;
/** /**
* Find the roots of the garbage collector. Each root is a pair * Find the roots of the garbage collector. Each root is a pair
* `(link, storepath)` where `link` is the path of the symlink * `(link, storepath)` where `link` is the path of the symlink

View file

@ -1,7 +1,6 @@
#include "derivations.hh" #include "derivations.hh"
#include "globals.hh" #include "globals.hh"
#include "local-store.hh" #include "local-store.hh"
#include "local-fs-store.hh"
#include "finally.hh" #include "finally.hh"
#include <functional> #include <functional>
@ -50,7 +49,7 @@ void LocalStore::addIndirectRoot(const Path & path)
} }
Path LocalFSStore::addPermRoot(const StorePath & storePath, const Path & _gcRoot) Path IndirectRootStore::addPermRoot(const StorePath & storePath, const Path & _gcRoot)
{ {
Path gcRoot(canonPath(_gcRoot)); Path gcRoot(canonPath(_gcRoot));

View file

@ -0,0 +1,48 @@
#pragma once
///@file
#include "local-fs-store.hh"
namespace nix {
/**
* Mix-in class for implementing permanent roots as a pair of a direct
* (strong) reference and indirect weak reference to the first
* reference.
*
* See methods for details on the operations it represents.
*/
struct IndirectRootStore : public virtual LocalFSStore
{
inline static std::string operationName = "Indirect GC roots registration";
/**
* Implementation of `LocalFSStore::addPermRoot` where the permanent
* root is a pair of
*
* - The user-facing symlink which all implementations must create
*
* - An additional weak reference known as the "indirect root" that
* points to that symlink.
*
* The garbage collector will automatically remove the indirect root
* when it finds that the symlink has disappeared.
*
* The implementation of this method is concrete, but it delegates
* to `addIndirectRoot()` which is abstract.
*/
Path addPermRoot(const StorePath & storePath, const Path & gcRoot) override final;
/**
* Add an indirect root, which is a weak reference to the
* user-facing symlink created by `addPermRoot()`.
*
* @param path user-facing and user-controlled symlink to a store
* path.
*
* The form this weak-reference takes is implementation-specific.
*/
virtual void addIndirectRoot(const Path & path) = 0;
};
}

View file

@ -40,6 +40,7 @@ class LocalFSStore : public virtual LocalFSStoreConfig,
public virtual LogStore public virtual LogStore
{ {
public: public:
inline static std::string operationName = "Local Filesystem Store";
const static std::string drvsLogDir; const static std::string drvsLogDir;
@ -49,9 +50,20 @@ public:
ref<FSAccessor> getFSAccessor() override; ref<FSAccessor> getFSAccessor() override;
/** /**
* Register a permanent GC root. * Creates symlink from the `gcRoot` to the `storePath` and
* registers the `gcRoot` as a permanent GC root. The `gcRoot`
* symlink lives outside the store and is created and owned by the
* user.
*
* @param gcRoot The location of the symlink.
*
* @param storePath The store object being rooted. The symlink will
* point to `toRealPath(store.printStorePath(storePath))`.
*
* How the permanent GC root corresponding to this symlink is
* managed is implementation-specific.
*/ */
Path addPermRoot(const StorePath & storePath, const Path & gcRoot); virtual Path addPermRoot(const StorePath & storePath, const Path & gcRoot) = 0;
virtual Path getRealStoreDir() { return realStoreDir; } virtual Path getRealStoreDir() { return realStoreDir; }

View file

@ -5,8 +5,7 @@
#include "pathlocks.hh" #include "pathlocks.hh"
#include "store-api.hh" #include "store-api.hh"
#include "local-fs-store.hh" #include "indirect-root-store.hh"
#include "gc-store.hh"
#include "sync.hh" #include "sync.hh"
#include "util.hh" #include "util.hh"
@ -68,7 +67,9 @@ struct LocalStoreConfig : virtual LocalFSStoreConfig
std::string doc() override; std::string doc() override;
}; };
class LocalStore : public virtual LocalStoreConfig, public virtual LocalFSStore, public virtual GcStore class LocalStore : public virtual LocalStoreConfig
, public virtual IndirectRootStore
, public virtual GcStore
{ {
private: private:
@ -209,6 +210,12 @@ private:
public: public:
/**
* Implementation of IndirectRootStore::addIndirectRoot().
*
* The weak reference merely is a symlink to `path' from
* /nix/var/nix/gcroots/auto/<hash of `path'>.
*/
void addIndirectRoot(const Path & path) override; void addIndirectRoot(const Path & path) override;
private: private:

View file

@ -1,5 +1,6 @@
#include "remote-store.hh" #include "remote-store.hh"
#include "worker-protocol.hh" #include "worker-protocol.hh"
#include "pool.hh"
namespace nix { namespace nix {
@ -94,4 +95,34 @@ struct RemoteStore::Connection
std::exception_ptr processStderr(Sink * sink = 0, Source * source = 0, bool flush = true); std::exception_ptr processStderr(Sink * sink = 0, Source * source = 0, bool flush = true);
}; };
/**
* A wrapper around Pool<RemoteStore::Connection>::Handle that marks
* the connection as bad (causing it to be closed) if a non-daemon
* exception is thrown before the handle is closed. Such an exception
* causes a deviation from the expected protocol and therefore a
* desynchronization between the client and daemon.
*/
struct RemoteStore::ConnectionHandle
{
Pool<RemoteStore::Connection>::Handle handle;
bool daemonException = false;
ConnectionHandle(Pool<RemoteStore::Connection>::Handle && handle)
: handle(std::move(handle))
{ }
ConnectionHandle(ConnectionHandle && h)
: handle(std::move(h.handle))
{ }
~ConnectionHandle();
RemoteStore::Connection & operator * () { return *handle; }
RemoteStore::Connection * operator -> () { return &*handle; }
void processStderr(Sink * sink = 0, Source * source = 0, bool flush = true);
void withFramedSink(std::function<void(Sink & sink)> fun);
};
} }

View file

@ -159,25 +159,7 @@ void RemoteStore::setOptions(Connection & conn)
} }
/* A wrapper around Pool<RemoteStore::Connection>::Handle that marks RemoteStore::ConnectionHandle::~ConnectionHandle()
the connection as bad (causing it to be closed) if a non-daemon
exception is thrown before the handle is closed. Such an exception
causes a deviation from the expected protocol and therefore a
desynchronization between the client and daemon. */
struct ConnectionHandle
{
Pool<RemoteStore::Connection>::Handle handle;
bool daemonException = false;
ConnectionHandle(Pool<RemoteStore::Connection>::Handle && handle)
: handle(std::move(handle))
{ }
ConnectionHandle(ConnectionHandle && h)
: handle(std::move(h.handle))
{ }
~ConnectionHandle()
{ {
if (!daemonException && std::uncaught_exceptions()) { if (!daemonException && std::uncaught_exceptions()) {
handle.markBad(); handle.markBad();
@ -185,10 +167,7 @@ struct ConnectionHandle
} }
} }
RemoteStore::Connection * operator -> () { return &*handle; } void RemoteStore::ConnectionHandle::processStderr(Sink * sink, Source * source, bool flush)
RemoteStore::Connection & operator * () { return *handle; }
void processStderr(Sink * sink = 0, Source * source = 0, bool flush = true)
{ {
auto ex = handle->processStderr(sink, source, flush); auto ex = handle->processStderr(sink, source, flush);
if (ex) { if (ex) {
@ -197,11 +176,8 @@ struct ConnectionHandle
} }
} }
void withFramedSink(std::function<void(Sink & sink)> fun);
};
RemoteStore::ConnectionHandle RemoteStore::getConnection()
ConnectionHandle RemoteStore::getConnection()
{ {
return ConnectionHandle(connections->get()); return ConnectionHandle(connections->get());
} }
@ -846,15 +822,6 @@ void RemoteStore::addTempRoot(const StorePath & path)
} }
void RemoteStore::addIndirectRoot(const Path & path)
{
auto conn(getConnection());
conn->to << WorkerProto::Op::AddIndirectRoot << path;
conn.processStderr();
readInt(conn->from);
}
Roots RemoteStore::findRoots(bool censor) Roots RemoteStore::findRoots(bool censor)
{ {
auto conn(getConnection()); auto conn(getConnection());
@ -1099,7 +1066,7 @@ std::exception_ptr RemoteStore::Connection::processStderr(Sink * sink, Source *
return nullptr; return nullptr;
} }
void ConnectionHandle::withFramedSink(std::function<void(Sink & sink)> fun) void RemoteStore::ConnectionHandle::withFramedSink(std::function<void(Sink & sink)> fun)
{ {
(*this)->to.flush(); (*this)->to.flush();

View file

@ -17,7 +17,6 @@ class Pid;
struct FdSink; struct FdSink;
struct FdSource; struct FdSource;
template<typename T> class Pool; template<typename T> class Pool;
struct ConnectionHandle;
struct RemoteStoreConfig : virtual StoreConfig struct RemoteStoreConfig : virtual StoreConfig
{ {
@ -127,8 +126,6 @@ public:
void addTempRoot(const StorePath & path) override; void addTempRoot(const StorePath & path) override;
void addIndirectRoot(const Path & path) override;
Roots findRoots(bool censor) override; Roots findRoots(bool censor) override;
void collectGarbage(const GCOptions & options, GCResults & results) override; void collectGarbage(const GCOptions & options, GCResults & results) override;
@ -182,6 +179,8 @@ protected:
void setOptions() override; void setOptions() override;
struct ConnectionHandle;
ConnectionHandle getConnection(); ConnectionHandle getConnection();
friend struct ConnectionHandle; friend struct ConnectionHandle;
@ -199,5 +198,4 @@ private:
std::shared_ptr<Store> evalStore); std::shared_ptr<Store> evalStore);
}; };
} }

View file

@ -1,5 +1,6 @@
#include "ssh-store-config.hh" #include "ssh-store-config.hh"
#include "store-api.hh" #include "store-api.hh"
#include "local-fs-store.hh"
#include "remote-store.hh" #include "remote-store.hh"
#include "remote-store-connection.hh" #include "remote-store-connection.hh"
#include "remote-fs-accessor.hh" #include "remote-fs-accessor.hh"
@ -61,7 +62,7 @@ public:
std::optional<std::string> getBuildLogExact(const StorePath & path) override std::optional<std::string> getBuildLogExact(const StorePath & path) override
{ unsupported("getBuildLogExact"); } { unsupported("getBuildLogExact"); }
private: protected:
struct Connection : RemoteStore::Connection struct Connection : RemoteStore::Connection
{ {
@ -93,9 +94,12 @@ private:
ref<RemoteStore::Connection> SSHStore::openConnection() ref<RemoteStore::Connection> SSHStore::openConnection()
{ {
auto conn = make_ref<Connection>(); auto conn = make_ref<Connection>();
conn->sshConn = master.startCommand(
fmt("%s --stdio", remoteProgram) std::string command = remoteProgram + " --stdio";
+ (remoteStore.get() == "" ? "" : " --store " + shellEscape(remoteStore.get()))); if (remoteStore.get() != "")
command += " --store " + shellEscape(remoteStore.get());
conn->sshConn = master.startCommand(command);
conn->to = FdSink(conn->sshConn->in.get()); conn->to = FdSink(conn->sshConn->in.get());
conn->from = FdSource(conn->sshConn->out.get()); conn->from = FdSource(conn->sshConn->out.get());
return conn; return conn;

View file

@ -99,6 +99,8 @@ typedef std::map<StorePath, std::optional<ContentAddress>> StorePathCAMap;
struct StoreConfig : public Config struct StoreConfig : public Config
{ {
typedef std::map<std::string, std::string> Params;
using Config::Config; using Config::Config;
StoreConfig() = delete; StoreConfig() = delete;
@ -153,10 +155,6 @@ struct StoreConfig : public Config
class Store : public std::enable_shared_from_this<Store>, public virtual StoreConfig class Store : public std::enable_shared_from_this<Store>, public virtual StoreConfig
{ {
public:
typedef std::map<std::string, std::string> Params;
protected: protected:
struct PathInfoCacheValue { struct PathInfoCacheValue {

View file

@ -1,4 +1,5 @@
#include "uds-remote-store.hh" #include "uds-remote-store.hh"
#include "worker-protocol.hh"
#include <sys/types.h> #include <sys/types.h>
#include <sys/stat.h> #include <sys/stat.h>
@ -77,6 +78,15 @@ ref<RemoteStore::Connection> UDSRemoteStore::openConnection()
} }
void UDSRemoteStore::addIndirectRoot(const Path & path)
{
auto conn(getConnection());
conn->to << WorkerProto::Op::AddIndirectRoot << path;
conn.processStderr();
readInt(conn->from);
}
static RegisterStoreImplementation<UDSRemoteStore, UDSRemoteStoreConfig> regUDSRemoteStore; static RegisterStoreImplementation<UDSRemoteStore, UDSRemoteStoreConfig> regUDSRemoteStore;
} }

View file

@ -3,13 +3,13 @@
#include "remote-store.hh" #include "remote-store.hh"
#include "remote-store-connection.hh" #include "remote-store-connection.hh"
#include "local-fs-store.hh" #include "indirect-root-store.hh"
namespace nix { namespace nix {
struct UDSRemoteStoreConfig : virtual LocalFSStoreConfig, virtual RemoteStoreConfig struct UDSRemoteStoreConfig : virtual LocalFSStoreConfig, virtual RemoteStoreConfig
{ {
UDSRemoteStoreConfig(const Store::Params & params) UDSRemoteStoreConfig(const Params & params)
: StoreConfig(params) : StoreConfig(params)
, LocalFSStoreConfig(params) , LocalFSStoreConfig(params)
, RemoteStoreConfig(params) , RemoteStoreConfig(params)
@ -21,7 +21,9 @@ struct UDSRemoteStoreConfig : virtual LocalFSStoreConfig, virtual RemoteStoreCon
std::string doc() override; std::string doc() override;
}; };
class UDSRemoteStore : public virtual UDSRemoteStoreConfig, public virtual LocalFSStore, public virtual RemoteStore class UDSRemoteStore : public virtual UDSRemoteStoreConfig
, public virtual IndirectRootStore
, public virtual RemoteStore
{ {
public: public:
@ -39,6 +41,16 @@ public:
void narFromPath(const StorePath & path, Sink & sink) override void narFromPath(const StorePath & path, Sink & sink) override
{ LocalFSStore::narFromPath(path, sink); } { LocalFSStore::narFromPath(path, sink); }
/**
* Implementation of `IndirectRootStore::addIndirectRoot()` which
* delegates to the remote store.
*
* The idea is that the client makes the direct symlink, so it is
* owned managed by the client's user account, and the server makes
* the indirect symlink.
*/
void addIndirectRoot(const Path & path) override;
private: private:
struct Connection : RemoteStore::Connection struct Connection : RemoteStore::Connection