Skip to content

Commit

Permalink
Combine the domain and key arguments into a single value for convenience
Browse files Browse the repository at this point in the history
  • Loading branch information
@edolstra
edolstra committed Apr 10, 2024
1 parent 1190b6d commit 661ebd3
Show file tree
Hide file tree
Showing 8 changed files with 84 additions and 99 deletions.
59 changes: 27 additions & 32 deletions src/libfetchers/cache.cc
View file
Expand Up @@ -51,57 +51,53 @@ struct CacheImpl : Cache
}

void upsert(
std::string_view domain,
const Attrs & key,
const Key & key,
const Attrs & value) override
{
_state.lock()->upsert.use()
(domain)
(attrsToJSON(key).dump())
(key.first)
(attrsToJSON(key.second).dump())
(attrsToJSON(value).dump())
(time(0)).exec();
}

std::optional<Attrs> lookup(
std::string_view domain,
const Attrs & key) override
const Key & key) override
{
if (auto res = lookupExpired(domain, key))
if (auto res = lookupExpired(key))
return std::move(res->value);
return {};
}

std::optional<Attrs> lookupWithTTL(
std::string_view domain,
const Attrs & key) override
const Key & key) override
{
if (auto res = lookupExpired(domain, key)) {
if (auto res = lookupExpired(key)) {
if (!res->expired)
return std::move(res->value);
debug("ignoring expired cache entry '%s:%s'",
domain, attrsToJSON(key).dump());
key.first, attrsToJSON(key.second).dump());
}
return {};
}

std::optional<Result> lookupExpired(
std::string_view domain,
const Attrs & key) override
const Key & key) override
{
auto state(_state.lock());

auto keyJSON = attrsToJSON(key).dump();
auto keyJSON = attrsToJSON(key.second).dump();

auto stmt(state->lookup.use()(domain)(keyJSON));
auto stmt(state->lookup.use()(key.first)(keyJSON));
if (!stmt.next()) {
debug("did not find cache entry for '%s:%s'", domain, keyJSON);
debug("did not find cache entry for '%s:%s'", key.first, keyJSON);
return {};
}

auto valueJSON = stmt.getStr(0);
auto timestamp = stmt.getInt(1);

debug("using cache entry '%s:%s' -> '%s'", domain, keyJSON, valueJSON);
debug("using cache entry '%s:%s' -> '%s'", key.first, keyJSON, valueJSON);

return Result {
.expired = settings.tarballTtl.get() == 0 || timestamp + settings.tarballTtl < time(0),
Expand All @@ -110,29 +106,27 @@ struct CacheImpl : Cache
}

void upsert(
std::string_view domain,
Attrs key,
Key key,
Store & store,
Attrs value,
const StorePath & storePath)
{
/* Add the store prefix to the cache key to handle multiple
store prefixes. */
key.insert_or_assign("store", store.storeDir);
key.second.insert_or_assign("store", store.storeDir);

value.insert_or_assign("storePath", (std::string) storePath.to_string());

upsert(domain, key, value);
upsert(key, value);
}

std::optional<ResultWithStorePath> lookupStorePath(
std::string_view domain,
Attrs key,
Key key,
Store & store) override
{
key.insert_or_assign("store", store.storeDir);
key.second.insert_or_assign("store", store.storeDir);

auto res = lookupExpired(domain, key);
auto res = lookupExpired(key);
if (!res) return std::nullopt;

auto storePathS = getStrAttr(res->value, "storePath");
Expand All @@ -143,26 +137,27 @@ struct CacheImpl : Cache
store.addTempRoot(res2.storePath);
if (!store.isValidPath(res2.storePath)) {
// FIXME: we could try to substitute 'storePath'.
debug("ignoring disappeared cache entry '%s' -> '%s'",
attrsToJSON(key).dump(),
debug("ignoring disappeared cache entry '%s:%s' -> '%s'",
key.first,
attrsToJSON(key.second).dump(),
store.printStorePath(res2.storePath));
return std::nullopt;
}

debug("using cache entry '%s' -> '%s', '%s'",
attrsToJSON(key).dump(),
debug("using cache entry '%s:%s' -> '%s', '%s'",
key.first,
attrsToJSON(key.second).dump(),
attrsToJSON(res2.value).dump(),
store.printStorePath(res2.storePath));

return res2;
}

std::optional<ResultWithStorePath> lookupStorePathWithTTL(
std::string_view domain,
Attrs key,
Key key,
Store & store) override
{
auto res = lookupStorePath(domain, std::move(key), store);
auto res = lookupStorePath(std::move(key), store);
return res && !res->expired ? res : std::nullopt;
}
};
Expand Down
35 changes: 19 additions & 16 deletions src/libfetchers/cache.hh
View file
Expand Up @@ -15,28 +15,35 @@ struct Cache
virtual ~Cache() { }

/**
* Add a value to the cache. The cache is an arbitrary mapping of
* Attrs to Attrs.
* A domain is a partition of the key/value cache for a particular
* purpose, e.g. "Git revision to revcount".
*/
using Domain = std::string_view;

/**
* A cache key is a domain and an arbitrary set of attributes.
*/
using Key = std::pair<Domain, Attrs>;

/**
* Add a key/value pair to the cache.
*/
virtual void upsert(
std::string_view domain,
const Attrs & key,
const Key & key,
const Attrs & value) = 0;

/**
* Look up a key with infinite TTL.
*/
virtual std::optional<Attrs> lookup(
std::string_view domain,
const Attrs & key) = 0;
const Key & key) = 0;

/**
* Look up a key. Return nothing if its TTL has exceeded
* `settings.tarballTTL`.
*/
virtual std::optional<Attrs> lookupWithTTL(
std::string_view domain,
const Attrs & key) = 0;
const Key & key) = 0;

struct Result
{
Expand All @@ -49,17 +56,15 @@ struct Cache
* exceeded `settings.tarballTTL`.
*/
virtual std::optional<Result> lookupExpired(
std::string_view domain,
const Attrs & key) = 0;
const Key & key) = 0;

/**
* Insert a cache entry that has a store path associated with
* it. Such cache entries are always considered stale if the
* associated store path is invalid.
*/
virtual void upsert(
std::string_view domain,
Attrs key,
Key key,
Store & store,
Attrs value,
const StorePath & storePath) = 0;
Expand All @@ -74,17 +79,15 @@ struct Cache
* be valid, but it may be expired.
*/
virtual std::optional<ResultWithStorePath> lookupStorePath(
std::string_view domain,
Attrs key,
Key key,
Store & store) = 0;

/**
* Look up a store path in the cache. Return nothing if its TTL
* has exceeded `settings.tarballTTL`.
*/
virtual std::optional<ResultWithStorePath> lookupStorePathWithTTL(
std::string_view domain,
Attrs key,
Key key,
Store & store) = 0;
};

Expand Down
11 changes: 5 additions & 6 deletions src/libfetchers/fetch-to-store.cc
View file
Expand Up @@ -16,17 +16,16 @@ StorePath fetchToStore(
// FIXME: add an optimisation for the case where the accessor is
// an FSInputAccessor pointing to a store path.

auto domain = "fetchToStore";
std::optional<fetchers::Attrs> cacheKey;
std::optional<fetchers::Cache::Key> cacheKey;

if (!filter && path.accessor->fingerprint) {
cacheKey = fetchers::Attrs{
cacheKey = fetchers::Cache::Key{"fetchToStore", {
{"name", std::string{name}},
{"fingerprint", *path.accessor->fingerprint},
{"method", std::string{method.render()}},
{"path", path.path.abs()}
};
if (auto res = fetchers::getCache()->lookupStorePath(domain, *cacheKey, store)) {
}};
if (auto res = fetchers::getCache()->lookupStorePath(*cacheKey, store)) {
debug("store path cache hit for '%s'", path);
return res->storePath;
}
Expand All @@ -46,7 +45,7 @@ StorePath fetchToStore(
name, *path.accessor, path.path, method, HashAlgorithm::SHA256, {}, filter2, repair);

if (cacheKey && mode == FetchMode::Copy)
fetchers::getCache()->upsert(domain, *cacheKey, store, {}, storePath);
fetchers::getCache()->upsert(*cacheKey, store, {}, storePath);

return storePath;
}
Expand Down
7 changes: 3 additions & 4 deletions src/libfetchers/git-utils.cc
View file
Expand Up @@ -456,15 +456,14 @@ struct GitRepoImpl : GitRepo, std::enable_shared_from_this<GitRepoImpl>
{
auto accessor = getAccessor(treeHash, false);

auto domain = "treeHashToNarHash";
fetchers::Attrs cacheKey({{"treeHash", treeHash.gitRev()}});
fetchers::Cache::Key cacheKey{"treeHashToNarHash", {{"treeHash", treeHash.gitRev()}}};

if (auto res = fetchers::getCache()->lookup(domain, cacheKey))
if (auto res = fetchers::getCache()->lookup(cacheKey))
return Hash::parseAny(fetchers::getStrAttr(*res, "narHash"), HashAlgorithm::SHA256);

auto narHash = accessor->hashPath(CanonPath::root);

fetchers::getCache()->upsert(domain, cacheKey, fetchers::Attrs({{"narHash", narHash.to_string(HashFormat::SRI, true)}}));
fetchers::getCache()->upsert(cacheKey, fetchers::Attrs({{"narHash", narHash.to_string(HashFormat::SRI, true)}}));

return narHash;
}
Expand Down
14 changes: 6 additions & 8 deletions src/libfetchers/github.cc
View file
Expand Up @@ -225,13 +225,11 @@ struct GitArchiveInputScheme : InputScheme

auto cache = getCache();

auto treeHashDomain = "gitRevToTreeHash";
Attrs treeHashKey{{"rev", rev->gitRev()}};
auto lastModifiedDomain = "gitRevToLastModified";
Attrs lastModifiedKey{{"rev", rev->gitRev()}};
Cache::Key treeHashKey{"gitRevToTreeHash", {{"rev", rev->gitRev()}}};
Cache::Key lastModifiedKey{"gitRevToLastModified", {{"rev", rev->gitRev()}}};

if (auto treeHashAttrs = cache->lookup(treeHashDomain, treeHashKey)) {
if (auto lastModifiedAttrs = cache->lookup(lastModifiedDomain, lastModifiedKey)) {
if (auto treeHashAttrs = cache->lookup(treeHashKey)) {
if (auto lastModifiedAttrs = cache->lookup(lastModifiedKey)) {
auto treeHash = getRevAttr(*treeHashAttrs, "treeHash");
auto lastModified = getIntAttr(*lastModifiedAttrs, "lastModified");
if (getTarballCache()->hasObject(treeHash))
Expand Down Expand Up @@ -259,8 +257,8 @@ struct GitArchiveInputScheme : InputScheme
.lastModified = lastModified
};

cache->upsert(treeHashDomain, treeHashKey, Attrs{{"treeHash", tarballInfo.treeHash.gitRev()}});
cache->upsert(lastModifiedDomain, lastModifiedKey, Attrs{{"lastModified", (uint64_t) tarballInfo.lastModified}});
cache->upsert(treeHashKey, Attrs{{"treeHash", tarballInfo.treeHash.gitRev()}});
cache->upsert(lastModifiedKey, Attrs{{"lastModified", (uint64_t) tarballInfo.lastModified}});

#if 0
if (upstreamTreeHash != tarballInfo.treeHash)
Expand Down
23 changes: 9 additions & 14 deletions src/libfetchers/tarball.cc
View file
Expand Up @@ -23,14 +23,12 @@ DownloadFileResult downloadFile(
{
// FIXME: check store

auto domain = "file";

Attrs key({
Cache::Key key{"file", {{
{"url", url},
{"name", name},
});
}}};

auto cached = getCache()->lookupStorePath(domain, key, *store);
auto cached = getCache()->lookupStorePath(key, *store);

auto useCached = [&]() -> DownloadFileResult
{
Expand Down Expand Up @@ -94,9 +92,9 @@ DownloadFileResult downloadFile(

/* Cache metadata for all URLs in the redirect chain. */
for (auto & url : res.urls) {
key.insert_or_assign("url", url);
key.second.insert_or_assign("url", url);
infoAttrs.insert_or_assign("url", *res.urls.rbegin());
getCache()->upsert(domain, key, *store, infoAttrs, *storePath);
getCache()->upsert(key, *store, infoAttrs, *storePath);
}

return {
Expand All @@ -111,12 +109,9 @@ DownloadTarballResult downloadTarball(
const std::string & url,
const Headers & headers)
{
auto domain = "tarball";
Attrs cacheKey{
{"url", url},
};
Cache::Key cacheKey{"tarball", {{"url", url}}};

auto cached = getCache()->lookupExpired(domain, cacheKey);
auto cached = getCache()->lookupExpired(cacheKey);

auto attrsToResult = [&](const Attrs & infoAttrs)
{
Expand Down Expand Up @@ -175,8 +170,8 @@ DownloadTarballResult downloadTarball(

/* Insert a cache entry for every URL in the redirect chain. */
for (auto & url : res->urls) {
cacheKey.insert_or_assign("url", url);
getCache()->upsert(domain, cacheKey, infoAttrs);
cacheKey.second.insert_or_assign("url", url);
getCache()->upsert(cacheKey, infoAttrs);
}

// FIXME: add a cache entry for immutableUrl? That could allow
Expand Down

0 comments on commit 661ebd3

Please sign in to comment.