diff --git a/doc/manual/rl-next/snix-http-cache-integration.md b/doc/manual/rl-next/snix-http-cache-integration.md new file mode 100644 index 000000000..a7c07a21b --- /dev/null +++ b/doc/manual/rl-next/snix-http-cache-integration.md @@ -0,0 +1,17 @@ +--- +synopsis: "libstore/binary-cache-store: don't cache narinfo on nix copy, remove negative entry" +issues: [] +cls: [3789] +category: Fixes +credits: [ma27] +--- + +When using e.g. [Snix's nar-bridge](https://snix.dev/docs/components/overview/#nar-bridge) via +an `http`-store, Lix would create cache entries with a wrong URL to the NAR when uploading +a store-path. + +This caused hard build failures for Hydra. + +Lix doesn't create these entries on upload anymore. Instead, it only removes negative cache entries. +The cache entry for a narinfo is now created the first time, Lix queries the cache +for the previously uploaded store-path again. diff --git a/lix/libstore/binary-cache-store.cc b/lix/libstore/binary-cache-store.cc index 5d4cbfcc8..5b24ba4a1 100644 --- a/lix/libstore/binary-cache-store.cc +++ b/lix/libstore/binary-cache-store.cc @@ -108,13 +108,12 @@ try { { auto state_(co_await state.lock()); - state_->pathInfoCache.upsert( - std::string(narInfo->path.to_string()), - PathInfoCacheValue { .value = std::shared_ptr(narInfo) }); + state_->pathInfoCache.erase(std::string(narInfo->path.to_string())); } - if (diskCache) - diskCache->upsertNarInfo(getUri(), std::string(narInfo->path.hashPart()), std::shared_ptr(narInfo)); + if (diskCache) { + diskCache->removeNegativeCacheEntry(getUri(), std::string(narInfo->path.hashPart())); + } co_return result::success(); } catch (...) { co_return result::current_exception(); diff --git a/lix/libstore/nar-info-disk-cache.cc b/lix/libstore/nar-info-disk-cache.cc index c4ddbda55..0e3bce2b1 100644 --- a/lix/libstore/nar-info-disk-cache.cc +++ b/lix/libstore/nar-info-disk-cache.cc @@ -71,8 +71,8 @@ public: struct State { SQLite db; - SQLiteStmt insertCache, queryCache, insertNAR, insertMissingNAR, - queryNAR, purgeCache; + SQLiteStmt insertCache, queryCache, insertNAR, insertMissingNAR, queryNAR, purgeCache, + removeNegativeCacheEntry; std::map caches; }; @@ -106,6 +106,9 @@ public: state->queryNAR = state->db.create( "select present, namePart, url, compression, fileHash, fileSize, narHash, narSize, refs, deriver, sigs, ca from NARs where cache = ? and hashPart = ? and ((present = 0 and timestamp > ?) or (present = 1 and timestamp > ?))"); + state->removeNegativeCacheEntry = + state->db.create("delete from NARs where present = 0 and hashPart = ? and cache = ?"); + /* Periodically purge expired entries from the database. */ retrySQLite([&]() { auto now = time(0); @@ -252,6 +255,18 @@ public: }, always_progresses); } + void removeNegativeCacheEntry(const std::string & uri, const std::string & hashPart) override + { + retrySQLite( + [&]() { + auto state(_state.lock()); + auto & cache(getCache(*state, uri)); + state->removeNegativeCacheEntry.use()(hashPart)(cache.id).exec(); + }, + always_progresses + ); + } + void upsertNarInfo( const std::string & uri, const std::string & hashPart, std::shared_ptr info) override diff --git a/lix/libstore/nar-info-disk-cache.hh b/lix/libstore/nar-info-disk-cache.hh index 4fa7e8a99..e0d152411 100644 --- a/lix/libstore/nar-info-disk-cache.hh +++ b/lix/libstore/nar-info-disk-cache.hh @@ -32,6 +32,9 @@ public: virtual void upsertNarInfo( const std::string & uri, const std::string & hashPart, std::shared_ptr info) = 0; + + virtual void + removeNegativeCacheEntry(const std::string & uri, const std::string & hashPart) = 0; }; /** diff --git a/tests/functional2/store/test_http.py b/tests/functional2/store/test_http.py new file mode 100644 index 000000000..f4b86d091 --- /dev/null +++ b/tests/functional2/store/test_http.py @@ -0,0 +1,149 @@ +from pathlib import Path +import sqlite3 + +import aiohttp.web as web +import pytest + +from functional2.testlib.fixtures.file_helper import File +from functional2.testlib.fixtures.http_server import http_server +from functional2.testlib.fixtures.nix import Nix + + +class HTTPStore: + def __init__(self): + self.uploaded_nars = {} + self.known_nar_hashes = set() + + async def upload_narinfo(self, req: web.Request) -> web.Response: + self.uploaded_nars[req.match_info["hash"]] = self._parse_narinfo(await req.text()) + return web.Response(text="") + + async def upload_nar(self, req: web.Request) -> web.Response: + self.known_nar_hashes.add(req.match_info["narhash"]) + return web.Response(text="") + + async def nix_cache_info(self, _: web.Request) -> web.Response: + return web.Response(text="StoreDir: /nix/store") + + async def get_narinfo(self, req: web.Request) -> web.Response: + narinfo_hash = req.match_info["hash"] + if narinfo_hash not in self.uploaded_nars: + return web.Response(text="", status=404) + return web.Response( + text="\n".join(f"{k}: {v}" for k, v in self.uploaded_nars[narinfo_hash].items()) + "\n" + ) + + async def nar_head(self, req: web.Request) -> web.Response: + return web.Response( + text="", status=200 if req.match_info["narhash"] in self.known_nar_hashes else 404 + ) + + def _parse_narinfo(self, text: str) -> dict[str, str]: + narinfo = {} + for line in text.splitlines(): + key, value = line.split(": ", 1) + narinfo[key] = value + _, hashpart = narinfo["FileHash"].split(":", 1) + assert hashpart in self.known_nar_hashes + return narinfo + + +class FakeNARBridge(HTTPStore): + """ + HTTP Store that mutates the URL field of the narinfo, just like + nar-bridge from snix.dev. + + Testcase to ensure that the correct URL to the nar (i.e. nar/snix-castore/...) + ends up in the disk-cache. + """ + + async def upload_narinfo(self, req: web.Request) -> web.Response: + narinfo = self._parse_narinfo(await req.text()) + narinfo["URL"] = ( + f"nar/snix-castore/00000000000000000000000000000000000000000000000000000?narsize=f{narinfo['FileSize']}" + ) + + self.uploaded_nars[req.match_info["hash"]] = narinfo + return web.Response(text="") + + +@pytest.fixture(params=[HTTPStore, FakeNARBridge]) +def store(request: pytest.FixtureRequest) -> HTTPStore: + store_class = request.param + return store_class() + + +def start_server(store: HTTPStore) -> web.Application: + app = web.Application() + app.add_routes( + [ + web.put("/{hash}.narinfo", store.upload_narinfo), + web.get("/{hash}.narinfo", store.get_narinfo), + web.put("/nar/{narhash}.nar", store.upload_nar), + web.get("/nix-cache-info", store.nix_cache_info), + web.head("/nar/{narhash}.nar", store.nar_head), + ] + ) + + return app + + +def nars_from_narinfo_cache(db_path: Path) -> list[dict[str, str | bool]]: + assert db_path.exists() + db = sqlite3.connect(db_path) + rows = db.execute( + "SELECT n.present, n.hashPart, n.namePart, n.url FROM NARs n INNER JOIN BinaryCaches b ON n.cache = b.id WHERE b.url LIKE '%localhost%'" + ) + return [ + {"present": bool(present), "hashPart": hashPart, "namePart": namePart, "url": url} + for present, hashPart, namePart, url in rows + ] + + +@pytest.mark.parametrize("files", [{"test-file": File("hello world")}], indirect=True) +def test_http_simple(nix: Nix, store: HTTPStore, files: Path): + test_file = files / "test-file" + result = nix.nix(cmd=["store", "add-file", test_file], flake=True).run() + result.ok() + store_path = result.stdout_plain + hash_part, _ = Path(store_path).stem.split("-", 1) + + nar_info_cache = nix.test_root / "test-home" / ".cache" / "nix" / "binary-cache-v6.sqlite" + + app = start_server(store) + with http_server(app) as httpd: + url = f"http://localhost:{httpd.port}?compression=none" + nix.nix(cmd=["store", "ping", "--store", url], flake=True).run().ok() + + # Narinfo shouldn't exist yet + nix.nix( + cmd=["path-info", "--store", f"http://localhost:{httpd.port}", store_path], flake=True + ).run().expect(1) + cache_entries = nars_from_narinfo_cache(nar_info_cache) + + assert len(cache_entries) == 1 + assert not cache_entries[0]["present"] + assert cache_entries[0]["hashPart"] == hash_part + + # Successful upload + nix.nix( + cmd=["copy", "--from", nix.test_root / "store", "--to", url, store_path], flake=True + ).run().ok() + assert hash_part in store.uploaded_nars + + # Make sure the negative entry got removed + assert not nars_from_narinfo_cache(nar_info_cache) + + # Ensure that the narinfo can be found now. + nix.nix( + cmd=["path-info", "--store", f"http://localhost:{httpd.port}", store_path], flake=True + ).run().ok() + + # Ensure local narinfo cache is up-to-date. + nar_entries = nars_from_narinfo_cache(nar_info_cache) + assert len(nar_entries) == 1 + + assert nar_entries[0]["present"] + assert nar_entries[0]["hashPart"] == hash_part + assert nar_entries[0]["namePart"] == "test-file" + assert nar_entries[0]["url"] == store.uploaded_nars[hash_part]["URL"] diff --git a/tests/functional2/testlib/fixtures/nix.py b/tests/functional2/testlib/fixtures/nix.py index f14f7e6ee..537aaf08f 100644 --- a/tests/functional2/testlib/fixtures/nix.py +++ b/tests/functional2/testlib/fixtures/nix.py @@ -104,6 +104,7 @@ class Nix: "NIX_DAEMON_SOCKET_PATH": self.test_root / "daemon-socket", "NIX_USER_CONF_FILES": "", "HOME": home, + "XDG_CACHE_HOME": home / ".cache", } def make_env(self) -> dict[AnyStr, AnyStr]: