From 1a86d3e98ebcc9a670c3c39499823298ce7fa1da Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 17 Jan 2023 19:52:19 +0100 Subject: [PATCH 1/7] local.mk: Don't log docroot comments These were accidentally logged and do not need to appear in make's log output. --- doc/manual/local.mk | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/manual/local.mk b/doc/manual/local.mk index 190f0258a..f43510b6d 100644 --- a/doc/manual/local.mk +++ b/doc/manual/local.mk @@ -51,13 +51,13 @@ $(d)/src/SUMMARY.md: $(d)/src/SUMMARY.md.in $(d)/src/command-ref/new-cli $(d)/src/command-ref/new-cli: $(d)/nix.json $(d)/generate-manpage.nix $(bindir)/nix @rm -rf $@ $(trace-gen) $(nix-eval) --write-to $@.tmp --expr 'import doc/manual/generate-manpage.nix { toplevel = builtins.readFile $<; }' - # @docroot@: https://nixos.org/manual/nix/unstable/contributing/hacking.html#docroot-variable + @# @docroot@: https://nixos.org/manual/nix/unstable/contributing/hacking.html#docroot-variable $(trace-gen) sed -i $@.tmp/*.md -e 's^@docroot@^../..^g' @mv $@.tmp $@ $(d)/src/command-ref/conf-file.md: $(d)/conf-file.json $(d)/generate-options.nix $(d)/src/command-ref/conf-file-prefix.md $(bindir)/nix @cat doc/manual/src/command-ref/conf-file-prefix.md > $@.tmp - # @docroot@: https://nixos.org/manual/nix/unstable/contributing/hacking.html#docroot-variable + @# @docroot@: https://nixos.org/manual/nix/unstable/contributing/hacking.html#docroot-variable $(trace-gen) $(nix-eval) --expr 'import doc/manual/generate-options.nix (builtins.fromJSON (builtins.readFile $<))' \ | sed -e 's^@docroot@^..^g'>> $@.tmp @mv $@.tmp $@ @@ -72,7 +72,7 @@ $(d)/conf-file.json: $(bindir)/nix $(d)/src/language/builtins.md: $(d)/builtins.json $(d)/generate-builtins.nix $(d)/src/language/builtins-prefix.md $(bindir)/nix @cat doc/manual/src/language/builtins-prefix.md > $@.tmp - # @docroot@: https://nixos.org/manual/nix/unstable/contributing/hacking.html#docroot-variable + @# @docroot@: https://nixos.org/manual/nix/unstable/contributing/hacking.html#docroot-variable $(trace-gen) $(nix-eval) --expr 'import doc/manual/generate-builtins.nix (builtins.fromJSON (builtins.readFile $<))' \ | sed -e 's^@docroot@^..^g' >> $@.tmp @cat doc/manual/src/language/builtins-suffix.md >> $@.tmp From 8a0ef5d58e0bb59eda0b8fd5622f2d731016f7a3 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 17 Jan 2023 19:53:21 +0100 Subject: [PATCH 2/7] sqlite.cc: Add SQL tracing Set environment variable NIX_DEBUG_SQLITE_TRACES=1 to log all sql statements. --- src/libstore/sqlite.cc | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/libstore/sqlite.cc b/src/libstore/sqlite.cc index 353dff9fa..871f2f3be 100644 --- a/src/libstore/sqlite.cc +++ b/src/libstore/sqlite.cc @@ -41,6 +41,15 @@ SQLiteError::SQLiteError(const char *path, const char *errMsg, int errNo, int ex throw SQLiteError(path, errMsg, err, exterr, offset, std::move(hf)); } +static void traceSQL(void * x, const char * sql) +{ + // wacky delimiters: + // so that we're quite unambiguous without escaping anything + // notice instead of trace: + // so that this can be enabled without getting the firehose in our face. + notice("SQL<[%1%]>", sql); +}; + SQLite::SQLite(const Path & path, bool create) { // useSQLiteWAL also indicates what virtual file system we need. Using @@ -58,6 +67,11 @@ SQLite::SQLite(const Path & path, bool create) if (sqlite3_busy_timeout(db, 60 * 60 * 1000) != SQLITE_OK) SQLiteError::throw_(db, "setting timeout"); + if (getEnv("NIX_DEBUG_SQLITE_TRACES") == "1") { + // To debug sqlite statements; trace all of them + sqlite3_trace(db, &traceSQL, nullptr); + } + exec("pragma foreign_keys = 1"); } From 29f0b196f44d273a5a85637168348c8a2e057049 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 17 Jan 2023 19:54:47 +0100 Subject: [PATCH 3/7] NarInfoDiskCache: Rename cacheExists -> upToDateCacheExists This is slightly more accurate considering that an outdated record may exist in the persistent cache. Possibly-outdated records are quite relevant as they may be foreign keys to more recent information that we want to keep, but we will not return them here. --- src/libstore/http-binary-cache-store.cc | 2 +- src/libstore/nar-info-disk-cache.cc | 2 +- src/libstore/nar-info-disk-cache.hh | 2 +- src/libstore/s3-binary-cache-store.cc | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libstore/http-binary-cache-store.cc b/src/libstore/http-binary-cache-store.cc index 73bcd6e81..1479822a9 100644 --- a/src/libstore/http-binary-cache-store.cc +++ b/src/libstore/http-binary-cache-store.cc @@ -56,7 +56,7 @@ public: void init() override { // FIXME: do this lazily? - if (auto cacheInfo = diskCache->cacheExists(cacheUri)) { + if (auto cacheInfo = diskCache->upToDateCacheExists(cacheUri)) { wantMassQuery.setDefault(cacheInfo->wantMassQuery); priority.setDefault(cacheInfo->priority); } else { diff --git a/src/libstore/nar-info-disk-cache.cc b/src/libstore/nar-info-disk-cache.cc index 3e0689534..883f364e5 100644 --- a/src/libstore/nar-info-disk-cache.cc +++ b/src/libstore/nar-info-disk-cache.cc @@ -200,7 +200,7 @@ public: }); } - std::optional cacheExists(const std::string & uri) override + std::optional upToDateCacheExists(const std::string & uri) override { return retrySQLite>([&]() -> std::optional { auto state(_state.lock()); diff --git a/src/libstore/nar-info-disk-cache.hh b/src/libstore/nar-info-disk-cache.hh index 2dcaa76a4..c185ca5e4 100644 --- a/src/libstore/nar-info-disk-cache.hh +++ b/src/libstore/nar-info-disk-cache.hh @@ -22,7 +22,7 @@ public: int priority; }; - virtual std::optional cacheExists(const std::string & uri) = 0; + virtual std::optional upToDateCacheExists(const std::string & uri) = 0; virtual std::pair> lookupNarInfo( const std::string & uri, const std::string & hashPart) = 0; diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index 844553ad3..8d76eee99 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -238,7 +238,7 @@ struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStoreConfig, public virtual void init() override { - if (auto cacheInfo = diskCache->cacheExists(getUri())) { + if (auto cacheInfo = diskCache->upToDateCacheExists(getUri())) { wantMassQuery.setDefault(cacheInfo->wantMassQuery); priority.setDefault(cacheInfo->priority); } else { From 79f62d2dda8603c1f2f471ce20557548db932296 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 30 Jan 2023 21:04:19 +0100 Subject: [PATCH 4/7] NarInfoDiskCacheImpl: Make dbPath a parameter This allows testing with a clean database. --- src/libstore/nar-info-disk-cache.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libstore/nar-info-disk-cache.cc b/src/libstore/nar-info-disk-cache.cc index 883f364e5..c460100de 100644 --- a/src/libstore/nar-info-disk-cache.cc +++ b/src/libstore/nar-info-disk-cache.cc @@ -84,11 +84,10 @@ public: Sync _state; - NarInfoDiskCacheImpl() + NarInfoDiskCacheImpl(Path dbPath = getCacheDir() + "/nix/binary-cache-v6.sqlite") { auto state(_state.lock()); - Path dbPath = getCacheDir() + "/nix/binary-cache-v6.sqlite"; createDirs(dirOf(dbPath)); state->db = SQLite(dbPath); From 2ceece3ef384385d886f6aed5311d9b6dbbdd6dd Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 30 Jan 2023 22:15:23 +0100 Subject: [PATCH 5/7] NarInfoDiskCache: Prepare reproducer for #3898 --- src/libstore/nar-info-disk-cache.cc | 6 ++ src/libstore/nar-info-disk-cache.hh | 3 + src/libstore/tests/nar-info-disk-cache.cc | 87 +++++++++++++++++++++++ 3 files changed, 96 insertions(+) create mode 100644 src/libstore/tests/nar-info-disk-cache.cc diff --git a/src/libstore/nar-info-disk-cache.cc b/src/libstore/nar-info-disk-cache.cc index c460100de..494318af9 100644 --- a/src/libstore/nar-info-disk-cache.cc +++ b/src/libstore/nar-info-disk-cache.cc @@ -207,6 +207,7 @@ public: if (!cache) return std::nullopt; return CacheInfo { + .id = cache->id, .wantMassQuery = cache->wantMassQuery, .priority = cache->priority }; @@ -370,4 +371,9 @@ ref getNarInfoDiskCache() return cache; } +ref getTestNarInfoDiskCache(Path dbPath) +{ + return make_ref(dbPath); +} + } diff --git a/src/libstore/nar-info-disk-cache.hh b/src/libstore/nar-info-disk-cache.hh index c185ca5e4..adc14f3bc 100644 --- a/src/libstore/nar-info-disk-cache.hh +++ b/src/libstore/nar-info-disk-cache.hh @@ -18,6 +18,7 @@ public: struct CacheInfo { + int id; bool wantMassQuery; int priority; }; @@ -45,4 +46,6 @@ public: multiple threads. */ ref getNarInfoDiskCache(); +ref getTestNarInfoDiskCache(Path dbPath); + } diff --git a/src/libstore/tests/nar-info-disk-cache.cc b/src/libstore/tests/nar-info-disk-cache.cc new file mode 100644 index 000000000..1e9cbaa78 --- /dev/null +++ b/src/libstore/tests/nar-info-disk-cache.cc @@ -0,0 +1,87 @@ +#include "nar-info-disk-cache.hh" + +#include +#include +#include "sqlite.hh" +#include + + +namespace nix { + +RC_GTEST_PROP( + NarInfoDiskCacheImpl, + create_and_read, + (int prio, bool wantMassQuery) + ) +{ + Path tmpDir = createTempDir(); + AutoDelete delTmpDir(tmpDir); + Path dbPath(tmpDir + "/test-narinfo-disk-cache.sqlite"); + auto cache = getTestNarInfoDiskCache(dbPath); + + cache->createCache("other://uri", "/nix/storedir", wantMassQuery, prio); + cache->createCache("other://uri-2", "/nix/storedir", wantMassQuery, prio); + + cache->createCache("the://uri", "/nix/storedir", wantMassQuery, prio); + + { + auto r = cache->upToDateCacheExists("the://uri"); + ASSERT_TRUE(r); + ASSERT_EQ(r->priority, prio); + ASSERT_EQ(r->wantMassQuery, wantMassQuery); + } + + SQLite db(dbPath); + SQLiteStmt getIds; + getIds.create(db, "SELECT id FROM BinaryCaches WHERE url = 'the://uri'"); + + int savedId; + { + auto q(getIds.use()); + ASSERT_TRUE(q.next()); + savedId = q.getInt(0); + ASSERT_FALSE(q.next()); + } + + + db.exec("UPDATE BinaryCaches SET timestamp = timestamp - 1 - 7 * 24 * 3600;"); + + // Relies on memory cache + { + auto r = cache->upToDateCacheExists("the://uri"); + ASSERT_TRUE(r); + ASSERT_EQ(r->priority, prio); + ASSERT_EQ(r->wantMassQuery, wantMassQuery); + } + + auto cache2 = getTestNarInfoDiskCache(dbPath); + + { + auto r = cache2->upToDateCacheExists("the://uri"); + ASSERT_FALSE(r); + } + + cache2->createCache("the://uri", "/nix/storedir", wantMassQuery, prio); + + { + auto r = cache->upToDateCacheExists("the://uri"); + ASSERT_TRUE(r); + ASSERT_EQ(r->priority, prio); + ASSERT_EQ(r->wantMassQuery, wantMassQuery); + // FIXME, reproduces #3898 + // ASSERT_EQ(r->id, savedId); + (void) savedId; + } + + { + auto q(getIds.use()); + ASSERT_TRUE(q.next()); + auto currentId = q.getInt(0); + ASSERT_FALSE(q.next()); + // FIXME, reproduces #3898 + // ASSERT_EQ(currentId, savedId); + (void) currentId; + } +} + +} From fb94d5cabd51d57aa82a9857ab20f5f4bd323378 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 17 Jan 2023 19:56:06 +0100 Subject: [PATCH 6/7] NarInfoDiskCache: Keep BinaryCache.id stable and improve test Fixes #3898 The entire `BinaryCaches` row used to get replaced after it became stale according to the `timestamp` column. In a concurrent scenario, this leads to foreign key conflicts as different instances of the in-process `state.caches` cache now differ, with the consequence that the older process still tries to use the `id` number of the old record. Furthermore, this phenomenon appears to have caused the cache for actual narinfos to be erased about every week, while the default ttl for narinfos was supposed to be 30 days. --- src/libstore/nar-info-disk-cache.cc | 38 ++++++++++--- src/libstore/nar-info-disk-cache.hh | 2 +- src/libstore/tests/nar-info-disk-cache.cc | 69 ++++++++++++++--------- 3 files changed, 73 insertions(+), 36 deletions(-) diff --git a/src/libstore/nar-info-disk-cache.cc b/src/libstore/nar-info-disk-cache.cc index 494318af9..2645f468b 100644 --- a/src/libstore/nar-info-disk-cache.cc +++ b/src/libstore/nar-info-disk-cache.cc @@ -97,7 +97,7 @@ public: state->db.exec(schema); state->insertCache.create(state->db, - "insert or replace into BinaryCaches(url, timestamp, storeDir, wantMassQuery, priority) values (?, ?, ?, ?, ?)"); + "insert into BinaryCaches(url, timestamp, storeDir, wantMassQuery, priority) values (?1, ?2, ?3, ?4, ?5) on conflict (url) do update set timestamp = ?2, storeDir = ?3, wantMassQuery = ?4, priority = ?5 returning id;"); state->queryCache.create(state->db, "select id, storeDir, wantMassQuery, priority from BinaryCaches where url = ? and timestamp > ?"); @@ -165,6 +165,8 @@ public: return i->second; } +private: + std::optional queryCacheRaw(State & state, const std::string & uri) { auto i = state.caches.find(uri); @@ -172,15 +174,21 @@ public: auto queryCache(state.queryCache.use()(uri)(time(0) - cacheInfoTtl)); if (!queryCache.next()) return std::nullopt; - state.caches.emplace(uri, - Cache{(int) queryCache.getInt(0), queryCache.getStr(1), queryCache.getInt(2) != 0, (int) queryCache.getInt(3)}); + auto cache = Cache { + .id = (int) queryCache.getInt(0), + .storeDir = queryCache.getStr(1), + .wantMassQuery = queryCache.getInt(2) != 0, + .priority = (int) queryCache.getInt(3), + }; + state.caches.emplace(uri, cache); } return getCache(state, uri); } - void createCache(const std::string & uri, const Path & storeDir, bool wantMassQuery, int priority) override +public: + int createCache(const std::string & uri, const Path & storeDir, bool wantMassQuery, int priority) override { - retrySQLite([&]() { + return retrySQLite([&]() { auto state(_state.lock()); SQLiteTxn txn(state->db); @@ -189,13 +197,25 @@ public: auto cache(queryCacheRaw(*state, uri)); if (cache) - return; + return cache->id; - state->insertCache.use()(uri)(time(0))(storeDir)(wantMassQuery)(priority).exec(); - assert(sqlite3_changes(state->db) == 1); - state->caches[uri] = Cache{(int) sqlite3_last_insert_rowid(state->db), storeDir, wantMassQuery, priority}; + Cache ret { + .id = -1, // set below + .storeDir = storeDir, + .wantMassQuery = wantMassQuery, + .priority = priority, + }; + + { + auto r(state->insertCache.use()(uri)(time(0))(storeDir)(wantMassQuery)(priority)); + assert(r.next()); + ret.id = (int) r.getInt(0); + } + + state->caches[uri] = ret; txn.commit(); + return ret.id; }); } diff --git a/src/libstore/nar-info-disk-cache.hh b/src/libstore/nar-info-disk-cache.hh index adc14f3bc..4877f56d8 100644 --- a/src/libstore/nar-info-disk-cache.hh +++ b/src/libstore/nar-info-disk-cache.hh @@ -13,7 +13,7 @@ public: virtual ~NarInfoDiskCache() { } - virtual void createCache(const std::string & uri, const Path & storeDir, + virtual int createCache(const std::string & uri, const Path & storeDir, bool wantMassQuery, int priority) = 0; struct CacheInfo diff --git a/src/libstore/tests/nar-info-disk-cache.cc b/src/libstore/tests/nar-info-disk-cache.cc index 1e9cbaa78..0597fdc41 100644 --- a/src/libstore/tests/nar-info-disk-cache.cc +++ b/src/libstore/tests/nar-info-disk-cache.cc @@ -8,69 +8,68 @@ namespace nix { -RC_GTEST_PROP( - NarInfoDiskCacheImpl, - create_and_read, - (int prio, bool wantMassQuery) - ) -{ +TEST(NarInfoDiskCacheImpl, create_and_read) { + int prio = 12345; + bool wantMassQuery = true; + Path tmpDir = createTempDir(); AutoDelete delTmpDir(tmpDir); Path dbPath(tmpDir + "/test-narinfo-disk-cache.sqlite"); auto cache = getTestNarInfoDiskCache(dbPath); - cache->createCache("other://uri", "/nix/storedir", wantMassQuery, prio); - cache->createCache("other://uri-2", "/nix/storedir", wantMassQuery, prio); - - cache->createCache("the://uri", "/nix/storedir", wantMassQuery, prio); - { - auto r = cache->upToDateCacheExists("the://uri"); + auto bc1 = cache->createCache("https://bar", "/nix/storedir", wantMassQuery, prio); + auto bc2 = cache->createCache("https://xyz", "/nix/storedir", false, 12); + ASSERT_NE(bc1, bc2); + } + + int savedId = cache->createCache("http://foo", "/nix/storedir", wantMassQuery, prio);; + { + auto r = cache->upToDateCacheExists("http://foo"); ASSERT_TRUE(r); ASSERT_EQ(r->priority, prio); ASSERT_EQ(r->wantMassQuery, wantMassQuery); + ASSERT_EQ(savedId, r->id); } SQLite db(dbPath); SQLiteStmt getIds; - getIds.create(db, "SELECT id FROM BinaryCaches WHERE url = 'the://uri'"); + getIds.create(db, "SELECT id FROM BinaryCaches WHERE url = 'http://foo'"); - int savedId; { auto q(getIds.use()); ASSERT_TRUE(q.next()); - savedId = q.getInt(0); + ASSERT_EQ(savedId, q.getInt(0)); ASSERT_FALSE(q.next()); } - db.exec("UPDATE BinaryCaches SET timestamp = timestamp - 1 - 7 * 24 * 3600;"); // Relies on memory cache { - auto r = cache->upToDateCacheExists("the://uri"); + auto r = cache->upToDateCacheExists("http://foo"); ASSERT_TRUE(r); ASSERT_EQ(r->priority, prio); ASSERT_EQ(r->wantMassQuery, wantMassQuery); } + // We can't clear the in-memory cache, so we use a new cache object. auto cache2 = getTestNarInfoDiskCache(dbPath); { - auto r = cache2->upToDateCacheExists("the://uri"); + auto r = cache2->upToDateCacheExists("http://foo"); ASSERT_FALSE(r); } - cache2->createCache("the://uri", "/nix/storedir", wantMassQuery, prio); + // Update, same data, check that the id number is reused + cache2->createCache("http://foo", "/nix/storedir", wantMassQuery, prio); { - auto r = cache->upToDateCacheExists("the://uri"); + auto r = cache2->upToDateCacheExists("http://foo"); ASSERT_TRUE(r); ASSERT_EQ(r->priority, prio); ASSERT_EQ(r->wantMassQuery, wantMassQuery); - // FIXME, reproduces #3898 - // ASSERT_EQ(r->id, savedId); - (void) savedId; + ASSERT_EQ(r->id, savedId); } { @@ -78,10 +77,28 @@ RC_GTEST_PROP( ASSERT_TRUE(q.next()); auto currentId = q.getInt(0); ASSERT_FALSE(q.next()); - // FIXME, reproduces #3898 - // ASSERT_EQ(currentId, savedId); - (void) currentId; + ASSERT_EQ(currentId, savedId); } + + // Check that the fields can be modified + { + auto r0 = cache2->upToDateCacheExists("https://bar"); + ASSERT_FALSE(r0); + + cache2->createCache("https://bar", "/nix/storedir", !wantMassQuery, prio + 10); + auto r = cache2->upToDateCacheExists("https://bar"); + ASSERT_EQ(r->wantMassQuery, !wantMassQuery); + ASSERT_EQ(r->priority, prio + 10); + } + + // // Force update (no use case yet; we only retrieve cache metadata when stale based on timestamp) + // { + // cache2->createCache("https://bar", "/nix/storedir", wantMassQuery, prio + 20); + // auto r = cache2->upToDateCacheExists("https://bar"); + // ASSERT_EQ(r->wantMassQuery, wantMassQuery); + // ASSERT_EQ(r->priority, prio + 20); + // } + } } From 19b495a48a45ac4f04d9b362c0bbaa6fc122c404 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 31 Jan 2023 15:46:54 +0100 Subject: [PATCH 7/7] NarInfoDiskCache: Also test id consistency with updated fields And clarify test --- src/libstore/tests/nar-info-disk-cache.cc | 159 ++++++++++++---------- 1 file changed, 89 insertions(+), 70 deletions(-) diff --git a/src/libstore/tests/nar-info-disk-cache.cc b/src/libstore/tests/nar-info-disk-cache.cc index 0597fdc41..b4bdb8329 100644 --- a/src/libstore/tests/nar-info-disk-cache.cc +++ b/src/libstore/tests/nar-info-disk-cache.cc @@ -9,96 +9,115 @@ namespace nix { TEST(NarInfoDiskCacheImpl, create_and_read) { + // This is a large single test to avoid some setup overhead. + int prio = 12345; bool wantMassQuery = true; Path tmpDir = createTempDir(); AutoDelete delTmpDir(tmpDir); Path dbPath(tmpDir + "/test-narinfo-disk-cache.sqlite"); - auto cache = getTestNarInfoDiskCache(dbPath); - { - auto bc1 = cache->createCache("https://bar", "/nix/storedir", wantMassQuery, prio); - auto bc2 = cache->createCache("https://xyz", "/nix/storedir", false, 12); - ASSERT_NE(bc1, bc2); - } - - int savedId = cache->createCache("http://foo", "/nix/storedir", wantMassQuery, prio);; - { - auto r = cache->upToDateCacheExists("http://foo"); - ASSERT_TRUE(r); - ASSERT_EQ(r->priority, prio); - ASSERT_EQ(r->wantMassQuery, wantMassQuery); - ASSERT_EQ(savedId, r->id); - } - - SQLite db(dbPath); + int savedId; + int barId; + SQLite db; SQLiteStmt getIds; - getIds.create(db, "SELECT id FROM BinaryCaches WHERE url = 'http://foo'"); { - auto q(getIds.use()); - ASSERT_TRUE(q.next()); - ASSERT_EQ(savedId, q.getInt(0)); - ASSERT_FALSE(q.next()); - } + auto cache = getTestNarInfoDiskCache(dbPath); - db.exec("UPDATE BinaryCaches SET timestamp = timestamp - 1 - 7 * 24 * 3600;"); + // Set up "background noise" and check that different caches receive different ids + { + auto bc1 = cache->createCache("https://bar", "/nix/storedir", wantMassQuery, prio); + auto bc2 = cache->createCache("https://xyz", "/nix/storedir", false, 12); + ASSERT_NE(bc1, bc2); + barId = bc1; + } - // Relies on memory cache - { - auto r = cache->upToDateCacheExists("http://foo"); - ASSERT_TRUE(r); - ASSERT_EQ(r->priority, prio); - ASSERT_EQ(r->wantMassQuery, wantMassQuery); - } + // Check that the fields are saved and returned correctly. This does not test + // the select statement yet, because of in-memory caching. + savedId = cache->createCache("http://foo", "/nix/storedir", wantMassQuery, prio);; + { + auto r = cache->upToDateCacheExists("http://foo"); + ASSERT_TRUE(r); + ASSERT_EQ(r->priority, prio); + ASSERT_EQ(r->wantMassQuery, wantMassQuery); + ASSERT_EQ(savedId, r->id); + } - // We can't clear the in-memory cache, so we use a new cache object. - auto cache2 = getTestNarInfoDiskCache(dbPath); + // We're going to pay special attention to the id field because we had a bug + // that changed it. + db = SQLite(dbPath); + getIds.create(db, "select id from BinaryCaches where url = 'http://foo'"); - { - auto r = cache2->upToDateCacheExists("http://foo"); - ASSERT_FALSE(r); - } + { + auto q(getIds.use()); + ASSERT_TRUE(q.next()); + ASSERT_EQ(savedId, q.getInt(0)); + ASSERT_FALSE(q.next()); + } - // Update, same data, check that the id number is reused - cache2->createCache("http://foo", "/nix/storedir", wantMassQuery, prio); + // Pretend that the caches are older, but keep one up to date, as "background noise" + db.exec("update BinaryCaches set timestamp = timestamp - 1 - 7 * 24 * 3600 where url <> 'https://xyz';"); - { - auto r = cache2->upToDateCacheExists("http://foo"); - ASSERT_TRUE(r); - ASSERT_EQ(r->priority, prio); - ASSERT_EQ(r->wantMassQuery, wantMassQuery); - ASSERT_EQ(r->id, savedId); + // This shows that the in-memory cache works + { + auto r = cache->upToDateCacheExists("http://foo"); + ASSERT_TRUE(r); + ASSERT_EQ(r->priority, prio); + ASSERT_EQ(r->wantMassQuery, wantMassQuery); + } } { - auto q(getIds.use()); - ASSERT_TRUE(q.next()); - auto currentId = q.getInt(0); - ASSERT_FALSE(q.next()); - ASSERT_EQ(currentId, savedId); + // We can't clear the in-memory cache, so we use a new cache object. This is + // more realistic anyway. + auto cache2 = getTestNarInfoDiskCache(dbPath); + + { + auto r = cache2->upToDateCacheExists("http://foo"); + ASSERT_FALSE(r); + } + + // "Update", same data, check that the id number is reused + cache2->createCache("http://foo", "/nix/storedir", wantMassQuery, prio); + + { + auto r = cache2->upToDateCacheExists("http://foo"); + ASSERT_TRUE(r); + ASSERT_EQ(r->priority, prio); + ASSERT_EQ(r->wantMassQuery, wantMassQuery); + ASSERT_EQ(r->id, savedId); + } + + { + auto q(getIds.use()); + ASSERT_TRUE(q.next()); + auto currentId = q.getInt(0); + ASSERT_FALSE(q.next()); + ASSERT_EQ(currentId, savedId); + } + + // Check that the fields can be modified, and the id remains the same + { + auto r0 = cache2->upToDateCacheExists("https://bar"); + ASSERT_FALSE(r0); + + cache2->createCache("https://bar", "/nix/storedir", !wantMassQuery, prio + 10); + auto r = cache2->upToDateCacheExists("https://bar"); + ASSERT_EQ(r->wantMassQuery, !wantMassQuery); + ASSERT_EQ(r->priority, prio + 10); + ASSERT_EQ(r->id, barId); + } + + // // Force update (no use case yet; we only retrieve cache metadata when stale based on timestamp) + // { + // cache2->createCache("https://bar", "/nix/storedir", wantMassQuery, prio + 20); + // auto r = cache2->upToDateCacheExists("https://bar"); + // ASSERT_EQ(r->wantMassQuery, wantMassQuery); + // ASSERT_EQ(r->priority, prio + 20); + // } } - - // Check that the fields can be modified - { - auto r0 = cache2->upToDateCacheExists("https://bar"); - ASSERT_FALSE(r0); - - cache2->createCache("https://bar", "/nix/storedir", !wantMassQuery, prio + 10); - auto r = cache2->upToDateCacheExists("https://bar"); - ASSERT_EQ(r->wantMassQuery, !wantMassQuery); - ASSERT_EQ(r->priority, prio + 10); - } - - // // Force update (no use case yet; we only retrieve cache metadata when stale based on timestamp) - // { - // cache2->createCache("https://bar", "/nix/storedir", wantMassQuery, prio + 20); - // auto r = cache2->upToDateCacheExists("https://bar"); - // ASSERT_EQ(r->wantMassQuery, wantMassQuery); - // ASSERT_EQ(r->priority, prio + 20); - // } - } }