Skip to content

Commit

Permalink
Merge pull request #7833 from hercules-ci/backport-7616-to-2.13-maint…
Browse files Browse the repository at this point in the history
…enance

Backport 7616 to 2.13 maintenance
  • Loading branch information
edolstra authored Feb 14, 2023
2 parents 911cd5b + 4ca48e3 commit 1083ecb
Show file tree
Hide file tree
Showing 7 changed files with 183 additions and 19 deletions.
6 changes: 3 additions & 3 deletions doc/manual/local.mk
Original file line number Diff line number Diff line change
Expand Up @@ -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 $@
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/libstore/http-binary-cache-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class HttpBinaryCacheStore : public virtual HttpBinaryCacheStoreConfig, public v
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 {
Expand Down
49 changes: 37 additions & 12 deletions src/libstore/nar-info-disk-cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,10 @@ class NarInfoDiskCacheImpl : public NarInfoDiskCache

Sync<State> _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);
Expand All @@ -98,7 +97,7 @@ class NarInfoDiskCacheImpl : public NarInfoDiskCache
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 > ?");
Expand Down Expand Up @@ -166,22 +165,30 @@ class NarInfoDiskCacheImpl : public NarInfoDiskCache
return i->second;
}

private:

std::optional<Cache> queryCacheRaw(State & state, const std::string & uri)
{
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;
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<void>([&]() {
return retrySQLite<int>([&]() {
auto state(_state.lock());
SQLiteTxn txn(state->db);

Expand All @@ -190,24 +197,37 @@ class NarInfoDiskCacheImpl : public NarInfoDiskCache
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;
});
}

std::optional<CacheInfo> cacheExists(const std::string & uri) override
std::optional<CacheInfo> upToDateCacheExists(const std::string & uri) override
{
return retrySQLite<std::optional<CacheInfo>>([&]() -> std::optional<CacheInfo> {
auto state(_state.lock());
auto cache(queryCacheRaw(*state, uri));
if (!cache)
return std::nullopt;
return CacheInfo {
.id = cache->id,
.wantMassQuery = cache->wantMassQuery,
.priority = cache->priority
};
Expand Down Expand Up @@ -371,4 +391,9 @@ ref<NarInfoDiskCache> getNarInfoDiskCache()
return cache;
}

ref<NarInfoDiskCache> getTestNarInfoDiskCache(Path dbPath)
{
return make_ref<NarInfoDiskCacheImpl>(dbPath);
}

}
7 changes: 5 additions & 2 deletions src/libstore/nar-info-disk-cache.hh
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,17 @@ 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
{
int id;
bool wantMassQuery;
int priority;
};

virtual std::optional<CacheInfo> cacheExists(const std::string & uri) = 0;
virtual std::optional<CacheInfo> upToDateCacheExists(const std::string & uri) = 0;

virtual std::pair<Outcome, std::shared_ptr<NarInfo>> lookupNarInfo(
const std::string & uri, const std::string & hashPart) = 0;
Expand All @@ -45,4 +46,6 @@ public:
multiple threads. */
ref<NarInfoDiskCache> getNarInfoDiskCache();

ref<NarInfoDiskCache> getTestNarInfoDiskCache(Path dbPath);

}
2 changes: 1 addition & 1 deletion src/libstore/s3-binary-cache-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
14 changes: 14 additions & 0 deletions src/libstore/sqlite.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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");
}

Expand Down
122 changes: 122 additions & 0 deletions src/libstore/tests/nar-info-disk-cache.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
#include "nar-info-disk-cache.hh"

#include <gtest/gtest.h>
#include "sqlite.hh"
#include <sqlite3.h>


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");

int savedId;
int barId;
SQLite db;
SQLiteStmt getIds;

{
auto cache = getTestNarInfoDiskCache(dbPath);

// 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;
}

// 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'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 q(getIds.use());
ASSERT_TRUE(q.next());
ASSERT_EQ(savedId, q.getInt(0));
ASSERT_FALSE(q.next());
}

// 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';");

// 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);
}
}

{
// 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);
// }
}
}

}

0 comments on commit 1083ecb

Please sign in to comment.