From 26dc468d8af0133e3f4ccc27d1f29d8bf7a1008c Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 17 Jan 2023 19:53:21 +0100 Subject: [PATCH 1/6] sqlite.cc: Add SQL tracing Set environment variable NIX_DEBUG_SQLITE_TRACES=1 to log all sql statements. (cherry picked from commit 8a0ef5d58e0bb59eda0b8fd5622f2d731016f7a3) (cherry picked from commit 54ac8fa919826482ac4eb0571291050a0223fe97) --- src/libstore/sqlite.cc | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/libstore/sqlite.cc b/src/libstore/sqlite.cc index 2090beabd..d238960b1 100644 --- a/src/libstore/sqlite.cc +++ b/src/libstore/sqlite.cc @@ -36,6 +36,15 @@ SQLiteError::SQLiteError(const char *path, int errNo, int extendedErrNo, hintfor throw SQLiteError(path, err, exterr, 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 @@ -49,6 +58,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 3b5a341c8cd8ad698b24207ce135ccbdcf3a4662 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 17 Jan 2023 19:54:47 +0100 Subject: [PATCH 2/6] 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. (cherry picked from commit 29f0b196f44d273a5a85637168348c8a2e057049) (cherry picked from commit 0bda735c579b0971d681c02e2e8bcfc948ab558a) --- 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 f4ea739b0..4b5b42952 100644 --- a/src/libstore/nar-info-disk-cache.cc +++ b/src/libstore/nar-info-disk-cache.cc @@ -179,7 +179,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 f6192b8c2375ecf302172f4a1cc1ebe6b7ae15c0 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 30 Jan 2023 21:04:19 +0100 Subject: [PATCH 3/6] NarInfoDiskCacheImpl: Make dbPath a parameter This allows testing with a clean database. (cherry picked from commit 79f62d2dda8603c1f2f471ce20557548db932296) (cherry picked from commit 4972085195802b4eb8d9b6ad98152f8b92adcadc) --- 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 4b5b42952..24b0861cd 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 6d6a836b315b9e7fdb88148494f33ed8920668ba Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 30 Jan 2023 22:15:23 +0100 Subject: [PATCH 4/6] NarInfoDiskCache: Prepare reproducer for #3898 (cherry picked from commit 2ceece3ef384385d886f6aed5311d9b6dbbdd6dd) (cherry picked from commit cabf1d9b40d423eb4bb8c425e7305d4576277011) --- src/libstore/nar-info-disk-cache.cc | 6 ++ src/libstore/nar-info-disk-cache.hh | 3 + src/libstore/tests/nar-info-disk-cache.cc | 86 +++++++++++++++++++++++ 3 files changed, 95 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 24b0861cd..94c7d7c2e 100644 --- a/src/libstore/nar-info-disk-cache.cc +++ b/src/libstore/nar-info-disk-cache.cc @@ -195,6 +195,7 @@ public: auto & cache(getCache(*state, uri)); return CacheInfo { + .id = cache.id, .wantMassQuery = cache.wantMassQuery, .priority = cache.priority }; @@ -358,4 +359,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..c3bc14fbb --- /dev/null +++ b/src/libstore/tests/nar-info-disk-cache.cc @@ -0,0 +1,86 @@ +#include "nar-info-disk-cache.hh" + +#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 112246045f4260ac0edbfd419886da98d49d55e4 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 17 Jan 2023 19:56:06 +0100 Subject: [PATCH 5/6] 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. (cherry picked from commit fb94d5cabd51d57aa82a9857ab20f5f4bd323378) (cherry picked from commit 8449b3cac3f2f383ec61bd9cd275d4aa156acee1) --- src/libstore/nar-info-disk-cache.cc | 55 +++++++++++++++--- src/libstore/nar-info-disk-cache.hh | 2 +- src/libstore/tests/nar-info-disk-cache.cc | 69 ++++++++++++++--------- 3 files changed, 92 insertions(+), 34 deletions(-) diff --git a/src/libstore/nar-info-disk-cache.cc b/src/libstore/nar-info-disk-cache.cc index 94c7d7c2e..b6ea71880 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,16 +165,57 @@ public: return i->second; } - void createCache(const std::string & uri, const Path & storeDir, bool wantMassQuery, int priority) override +private: + + std::optional queryCacheRaw(State & state, const std::string & uri) { - retrySQLite([&]() { + auto i = state.caches.find(uri); + if (i == state.caches.end()) { + auto queryCache(state.queryCache.use()(uri)(time(0) - cacheInfoTtl)); + if (!queryCache.next()) + return std::nullopt; + 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); + } + +public: + int createCache(const std::string & uri, const Path & storeDir, bool wantMassQuery, int priority) override + { + return retrySQLite([&]() { auto state(_state.lock()); + SQLiteTxn txn(state->db); - // FIXME: race + // To avoid the race, we have to check if maybe someone hasn't yet created + // the cache for this URI in the meantime. + auto cache(queryCacheRaw(*state, uri)); - 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}; + if (cache) + return cache->id; + + 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 c3bc14fbb..971a251af 100644 --- a/src/libstore/tests/nar-info-disk-cache.cc +++ b/src/libstore/tests/nar-info-disk-cache.cc @@ -7,69 +7,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); } { @@ -77,10 +76,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 48b407c23ed84bcf3c4a4b51157160bb1cf8a24b Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 31 Jan 2023 15:46:54 +0100 Subject: [PATCH 6/6] NarInfoDiskCache: Also test id consistency with updated fields And clarify test (cherry picked from commit 19b495a48a45ac4f04d9b362c0bbaa6fc122c404) (cherry picked from commit 26378d285b45ef6177c4e7c87d4e72e927038bd2) --- 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 971a251af..17825f845 100644 --- a/src/libstore/tests/nar-info-disk-cache.cc +++ b/src/libstore/tests/nar-info-disk-cache.cc @@ -8,96 +8,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); - // } - } }