From 7d14f5c3310f5380ca14391e79bd1fc214d5f5c9 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 21 Apr 2016 17:53:47 +0200 Subject: [PATCH] Implement S3BinaryCacheStore::queryAllValidPaths() This allows commands like "nix verify --all" or "nix path-info --all" to work on S3 caches. Unfortunately, this requires some ugly hackery: when querying the contents of the bucket, we don't want to have to read every .narinfo file. But the S3 bucket keys only include the hash part of each store path, not the name part. So as a special exception queryAllValidPaths() can now return store paths *without* the name part, and queryPathInfo() accepts such store paths (returning a ValidPathInfo object containing the full name). --- src/libstore/binary-cache-store.cc | 8 ++--- src/libstore/local-store.cc | 4 +-- src/libstore/nar-info-disk-cache.cc | 51 +++++++++++++++------------ src/libstore/nar-info-disk-cache.hh | 5 +-- src/libstore/s3-binary-cache-store.cc | 35 +++++++++++++++++- src/libstore/store-api.cc | 34 ++++++++++++------ src/libstore/store-api.hh | 11 ++++-- src/nix/path-info.cc | 11 +++--- src/nix/verify.cc | 8 ++--- 9 files changed, 113 insertions(+), 54 deletions(-) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index dacdeb3f5..4445bfd27 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -94,13 +94,15 @@ void BinaryCacheStore::addToCache(const ValidPathInfo & info, upsertFile(narInfoFile, narInfo->to_string()); + auto hashPart = storePathToHash(narInfo->path); + { auto state_(state.lock()); - state_->pathInfoCache.upsert(narInfo->path, std::shared_ptr(narInfo)); + state_->pathInfoCache.upsert(hashPart, std::shared_ptr(narInfo)); } if (diskCache) - diskCache->upsertNarInfo(getUri(), std::shared_ptr(narInfo)); + diskCache->upsertNarInfo(getUri(), hashPart, std::shared_ptr(narInfo)); stats.narInfoWrite++; } @@ -197,8 +199,6 @@ std::shared_ptr BinaryCacheStore::queryPathInfoUncached(const Pat if (!data) return 0; auto narInfo = make_ref(*data, narInfoFile); - if (narInfo->path != storePath) - throw Error(format("NAR info file for store path ‘%1%’ does not match ‘%2%’") % narInfo->path % storePath); stats.narInfoRead++; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 9bc164e19..baf3a528b 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -579,7 +579,7 @@ uint64_t LocalStore::addValidPath(State & state, { auto state_(Store::state.lock()); - state_->pathInfoCache.upsert(info.path, std::make_shared(info)); + state_->pathInfoCache.upsert(storePathToHash(info.path), std::make_shared(info)); } return id; @@ -1067,7 +1067,7 @@ void LocalStore::invalidatePath(State & state, const Path & path) { auto state_(Store::state.lock()); - state_->pathInfoCache.erase(path); + state_->pathInfoCache.erase(storePathToHash(path)); } } diff --git a/src/libstore/nar-info-disk-cache.cc b/src/libstore/nar-info-disk-cache.cc index 30ef7b36c..d8b0815bf 100644 --- a/src/libstore/nar-info-disk-cache.cc +++ b/src/libstore/nar-info-disk-cache.cc @@ -20,7 +20,8 @@ create table if not exists BinaryCaches ( create table if not exists NARs ( cache integer not null, - storePath text not null, + hashPart text not null, + namePart text not null, url text, compression text, fileHash text, @@ -31,7 +32,7 @@ create table if not exists NARs ( deriver text, sigs text, timestamp integer not null, - primary key (cache, storePath), + primary key (cache, hashPart), foreign key (cache) references BinaryCaches(id) on delete cascade ); @@ -66,7 +67,7 @@ public: { auto state(_state.lock()); - Path dbPath = getCacheDir() + "/nix/binary-cache-v3.sqlite"; + Path dbPath = getCacheDir() + "/nix/binary-cache-v4.sqlite"; createDirs(dirOf(dbPath)); if (sqlite3_open_v2(dbPath.c_str(), &state->db.db, @@ -92,11 +93,11 @@ public: "select id, storeDir, wantMassQuery, priority from BinaryCaches where url = ?"); state->insertNAR.create(state->db, - "insert or replace into NARs(cache, storePath, url, compression, fileHash, fileSize, narHash, " - "narSize, refs, deriver, sigs, timestamp) values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"); + "insert or replace into NARs(cache, hashPart, namePart, url, compression, fileHash, fileSize, narHash, " + "narSize, refs, deriver, sigs, timestamp) values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"); state->queryNAR.create(state->db, - "select * from NARs where cache = ? and storePath = ?"); + "select * from NARs where cache = ? and hashPart = ?"); state->insertNARExistence.create(state->db, "insert or replace into NARExistence(cache, storePath, exist, timestamp) values (?, ?, ?, ?)"); @@ -141,13 +142,13 @@ public: } std::pair> lookupNarInfo( - const std::string & uri, const Path & storePath) override + const std::string & uri, const std::string & hashPart) override { auto state(_state.lock()); auto queryNAR(state->queryNAR.use() (uriToInt(*state, uri)) - (baseNameOf(storePath))); + (hashPart)); if (!queryNAR.next()) // FIXME: check NARExistence @@ -157,26 +158,29 @@ public: // FIXME: implement TTL. - narInfo->path = storePath; - narInfo->url = queryNAR.getStr(2); - narInfo->compression = queryNAR.getStr(3); - if (!queryNAR.isNull(4)) - narInfo->fileHash = parseHash(queryNAR.getStr(4)); - narInfo->fileSize = queryNAR.getInt(5); - narInfo->narHash = parseHash(queryNAR.getStr(6)); - narInfo->narSize = queryNAR.getInt(7); - for (auto & r : tokenizeString(queryNAR.getStr(8), " ")) + auto namePart = queryNAR.getStr(2); + narInfo->path = settings.nixStore + "/" + + hashPart + (namePart.empty() ? "" : "-" + namePart); + narInfo->url = queryNAR.getStr(3); + narInfo->compression = queryNAR.getStr(4); + if (!queryNAR.isNull(5)) + narInfo->fileHash = parseHash(queryNAR.getStr(5)); + narInfo->fileSize = queryNAR.getInt(6); + narInfo->narHash = parseHash(queryNAR.getStr(7)); + narInfo->narSize = queryNAR.getInt(8); + for (auto & r : tokenizeString(queryNAR.getStr(9), " ")) narInfo->references.insert(settings.nixStore + "/" + r); - if (!queryNAR.isNull(9)) - narInfo->deriver = settings.nixStore + "/" + queryNAR.getStr(9); - for (auto & sig : tokenizeString(queryNAR.getStr(10), " ")) + if (!queryNAR.isNull(10)) + narInfo->deriver = settings.nixStore + "/" + queryNAR.getStr(10); + for (auto & sig : tokenizeString(queryNAR.getStr(11), " ")) narInfo->sigs.insert(sig); return {oValid, narInfo}; } void upsertNarInfo( - const std::string & uri, std::shared_ptr info) override + const std::string & uri, const std::string & hashPart, + std::shared_ptr info) override { auto state(_state.lock()); @@ -184,9 +188,12 @@ public: auto narInfo = std::dynamic_pointer_cast(info); + assert(hashPart == storePathToHash(info->path)); + state->insertNAR.use() (uriToInt(*state, uri)) - (baseNameOf(info->path)) + (hashPart) + (storePathToName(info->path)) (narInfo ? narInfo->url : "", narInfo != 0) (narInfo ? narInfo->compression : "", narInfo != 0) (narInfo && narInfo->fileHash ? narInfo->fileHash.to_string() : "", narInfo && narInfo->fileHash) diff --git a/src/libstore/nar-info-disk-cache.hh b/src/libstore/nar-info-disk-cache.hh index ee1aafc63..f4e3fbbdc 100644 --- a/src/libstore/nar-info-disk-cache.hh +++ b/src/libstore/nar-info-disk-cache.hh @@ -15,10 +15,11 @@ public: virtual bool cacheExists(const std::string & uri) = 0; virtual std::pair> lookupNarInfo( - const std::string & uri, const Path & storePath) = 0; + const std::string & uri, const std::string & hashPart) = 0; virtual void upsertNarInfo( - const std::string & uri, std::shared_ptr narInfo) = 0; + const std::string & uri, const std::string & hashPart, + std::shared_ptr info) = 0; }; /* Return a singleton cache object that can be used concurrently by diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index 9ac79cf41..6fadf7be2 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -10,6 +10,7 @@ #include #include #include +#include namespace nix { @@ -164,7 +165,7 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore std::shared_ptr getFile(const std::string & path) { - printMsg(lvlDebug, format("fetching ‘s3://%1%/%2%’...") % bucketName % path); + debug(format("fetching ‘s3://%1%/%2%’...") % bucketName % path); auto request = Aws::S3::Model::GetObjectRequest() @@ -204,6 +205,38 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore } } + PathSet queryAllValidPaths() override + { + PathSet paths; + std::string marker; + + do { + debug(format("listing bucket ‘s3://%s’ from key ‘%s’...") % bucketName % marker); + + auto res = checkAws(format("AWS error listing bucket ‘%s’") % bucketName, + client->ListObjects( + Aws::S3::Model::ListObjectsRequest() + .WithBucket(bucketName) + .WithDelimiter("/") + .WithMarker(marker))); + + auto & contents = res.GetContents(); + + debug(format("got %d keys, next marker ‘%s’") + % contents.size() % res.GetNextMarker()); + + for (auto object : contents) { + auto & key = object.GetKey(); + if (!hasSuffix(key, ".narinfo")) continue; + paths.insert(settings.nixStore + "/" + std::string(key, 0, key.size() - 8)); + } + + marker = res.GetNextMarker(); + } while (!marker.empty()); + + return paths; + } + }; static RegisterStoreImplementation regStore([](const std::string & uri) -> std::shared_ptr { diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index cac137a99..9870e36b5 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -63,13 +63,16 @@ Path followLinksToStorePath(const Path & path) string storePathToName(const Path & path) { assertStorePath(path); - return string(path, settings.nixStore.size() + storePathHashLen + 2); + auto l = settings.nixStore.size() + 1 + storePathHashLen; + assert(path.size() >= l); + return path.size() == l ? "" : string(path, l + 1); } string storePathToHash(const Path & path) { assertStorePath(path); + assert(path.size() >= settings.nixStore.size() + 1 + storePathHashLen); return string(path, settings.nixStore.size() + 1, storePathHashLen); } @@ -234,9 +237,11 @@ std::string Store::getUri() bool Store::isValidPath(const Path & storePath) { + auto hashPart = storePathToHash(storePath); + { auto state_(state.lock()); - auto res = state_->pathInfoCache.get(storePath); + auto res = state_->pathInfoCache.get(hashPart); if (res) { stats.narInfoReadAverted++; return *res != 0; @@ -244,10 +249,11 @@ bool Store::isValidPath(const Path & storePath) } if (diskCache) { - auto res = diskCache->lookupNarInfo(getUri(), storePath); + auto res = diskCache->lookupNarInfo(getUri(), hashPart); if (res.first != NarInfoDiskCache::oUnknown) { + stats.narInfoReadAverted++; auto state_(state.lock()); - state_->pathInfoCache.upsert(storePath, + state_->pathInfoCache.upsert(hashPart, res.first == NarInfoDiskCache::oInvalid ? 0 : res.second); return res.first == NarInfoDiskCache::oValid; } @@ -261,9 +267,11 @@ bool Store::isValidPath(const Path & storePath) ref Store::queryPathInfo(const Path & storePath) { + auto hashPart = storePathToHash(storePath); + { auto state_(state.lock()); - auto res = state_->pathInfoCache.get(storePath); + auto res = state_->pathInfoCache.get(hashPart); if (res) { stats.narInfoReadAverted++; if (!*res) @@ -273,12 +281,14 @@ ref Store::queryPathInfo(const Path & storePath) } if (diskCache) { - auto res = diskCache->lookupNarInfo(getUri(), storePath); + auto res = diskCache->lookupNarInfo(getUri(), hashPart); if (res.first != NarInfoDiskCache::oUnknown) { + stats.narInfoReadAverted++; auto state_(state.lock()); - state_->pathInfoCache.upsert(storePath, + state_->pathInfoCache.upsert(hashPart, res.first == NarInfoDiskCache::oInvalid ? 0 : res.second); - if (res.first == NarInfoDiskCache::oInvalid) + if (res.first == NarInfoDiskCache::oInvalid || + (res.second->path != storePath && storePathToName(storePath) != "")) throw InvalidPath(format("path ‘%s’ is not valid") % storePath); return ref(res.second); } @@ -287,14 +297,16 @@ ref Store::queryPathInfo(const Path & storePath) auto info = queryPathInfoUncached(storePath); if (diskCache && info) - diskCache->upsertNarInfo(getUri(), info); + diskCache->upsertNarInfo(getUri(), hashPart, info); { auto state_(state.lock()); - state_->pathInfoCache.upsert(storePath, info); + state_->pathInfoCache.upsert(hashPart, info); } - if (!info) { + if (!info + || (info->path != storePath && storePathToName(storePath) != "")) + { stats.narInfoMissing++; throw InvalidPath(format("path ‘%s’ is not valid") % storePath); } diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index eab6da91e..e0cc32296 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -181,7 +181,7 @@ protected: struct State { - LRUCache> pathInfoCache{64 * 1024}; + LRUCache> pathInfoCache{64 * 1024}; }; Sync state; @@ -206,10 +206,15 @@ public: /* Query which of the given paths is valid. */ virtual PathSet queryValidPaths(const PathSet & paths) = 0; - /* Query the set of all valid paths. */ + /* Query the set of all valid paths. Note that for some store + backends, the name part of store paths may be omitted + (i.e. you'll get /nix/store/ rather than + /nix/store/-). Use queryPathInfo() to obtain the + full store path. */ virtual PathSet queryAllValidPaths() = 0; - /* Query information about a valid path. */ + /* Query information about a valid path. It is permitted to omit + the name part of the store path. */ ref queryPathInfo(const Path & path); protected: diff --git a/src/nix/path-info.cc b/src/nix/path-info.cc index 8497d708c..d144ef082 100644 --- a/src/nix/path-info.cc +++ b/src/nix/path-info.cc @@ -4,6 +4,7 @@ #include "store-api.hh" #include +#include using namespace nix; @@ -48,14 +49,14 @@ struct CmdPathInfo : StorePathsCommand for (auto & storePath : storePaths) pathLen = std::max(pathLen, storePath.size()); - for (auto & storePath : storePaths) { - if (!store->isValidPath(storePath)) - throw Error(format("path ‘%s’ is not valid") % storePath); + for (auto storePath : storePaths) { + auto info = store->queryPathInfo(storePath); + storePath = info->path; // FIXME: screws up padding - std::cout << storePath << std::string(pathLen - storePath.size(), ' '); + std::cout << storePath << std::string(std::max(0, (int) pathLen - (int) storePath.size()), ' '); if (showSize) { - std::cout << '\t' << std::setw(11) << store->queryPathInfo(storePath)->narSize; + std::cout << '\t' << std::setw(11) << info->narSize; } if (showClosureSize) { diff --git a/src/nix/verify.cc b/src/nix/verify.cc index fdbc2b0fd..3844535e7 100644 --- a/src/nix/verify.cc +++ b/src/nix/verify.cc @@ -98,7 +98,7 @@ struct CmdVerify : StorePathsCommand if (!noContents) { HashSink sink(info->narHash.type); - store->narFromPath(storePath, sink); + store->narFromPath(info->path, sink); auto hash = sink.finish(); @@ -106,7 +106,7 @@ struct CmdVerify : StorePathsCommand corrupted = 1; printMsg(lvlError, format("path ‘%s’ was modified! expected hash ‘%s’, got ‘%s’") - % storePath % printHash(info->narHash) % printHash(hash.first)); + % info->path % printHash(info->narHash) % printHash(hash.first)); } } @@ -138,7 +138,7 @@ struct CmdVerify : StorePathsCommand for (auto & store2 : substituters) { if (validSigs >= actualSigsNeeded) break; try { - doSigs(store2->queryPathInfo(storePath)->sigs); + doSigs(store2->queryPathInfo(info->path)->sigs); } catch (InvalidPath &) { } catch (Error & e) { printMsg(lvlError, format(ANSI_RED "error:" ANSI_NORMAL " %s") % e.what()); @@ -151,7 +151,7 @@ struct CmdVerify : StorePathsCommand if (!good) { untrusted++; - printMsg(lvlError, format("path ‘%s’ is untrusted") % storePath); + printMsg(lvlError, format("path ‘%s’ is untrusted") % info->path); } }