From c76f0467b2d13d5d53b3418a32d58d4dcb405861 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sun, 15 Jun 2025 14:47:08 +0200 Subject: [PATCH] libstore: asyncify BinaryCacheStore::fileExists Change-Id: I7574f61bf222389606be87bbaff486b386cdbecd --- lix/libstore/binary-cache-store.cc | 21 +++++++++++++++------ lix/libstore/binary-cache-store.hh | 2 +- lix/libstore/http-binary-cache-store.cc | 8 +++++--- lix/libstore/local-binary-cache-store.cc | 10 ++++++---- lix/libstore/s3-binary-cache-store.cc | 10 ++++++---- 5 files changed, 33 insertions(+), 18 deletions(-) diff --git a/lix/libstore/binary-cache-store.cc b/lix/libstore/binary-cache-store.cc index 7e4731739..8e89335b7 100644 --- a/lix/libstore/binary-cache-store.cc +++ b/lix/libstore/binary-cache-store.cc @@ -19,6 +19,7 @@ #include "lix/libutil/strings.hh" #include +#include #include #include #include @@ -214,7 +215,10 @@ try { ThreadPool threadPool("write debuginfo pool", 25); - auto doFile = [&](std::string member, std::string key, std::string target) { + auto doFile = [&](AsyncIoRoot & aio, + std::string member, + std::string key, + std::string target) { checkInterrupt(); JSON json; @@ -223,7 +227,9 @@ try { // FIXME: or should we overwrite? The previous link may point // to a GC'ed file, so overwriting might be useful... - if (fileExists(key)) return; + if (aio.blockOn(fileExists(key))) { + return; + } printMsg(lvlTalkative, "creating debuginfo link from '%s' to '%s'", key, target); @@ -251,7 +257,9 @@ try { std::string key = "debuginfo/" + buildId; std::string target = "../" + narInfo->url; - threadPool.enqueue(std::bind(doFile, std::string(debugPath, 1), key, target)); + threadPool.enqueueWithAio(std::bind( + doFile, std::placeholders::_1, std::string(debugPath, 1), key, target + )); } } @@ -260,13 +268,14 @@ try { } /* Atomically write the NAR file. */ - if (repair || !fileExists(narInfo->url)) { + if (repair || !TRY_AWAIT(fileExists(narInfo->url))) { stats.narWrite++; upsertFile(narInfo->url, std::make_shared(fnTemp, std::ios_base::in | std::ios_base::binary), "application/x-nix-nar"); - } else + } else { stats.narWriteAverted++; + } stats.narWriteBytes += info.narSize; stats.narWriteCompressedBytes += fileSize; @@ -347,7 +356,7 @@ try { // FIXME: this only checks whether a .narinfo with a matching hash // part exists. So ‘f4kb...-foo’ matches ‘f4kb...-bar’, even // though they shouldn't. Not easily fixed. - co_return fileExists(narInfoFileFor(storePath)); + co_return TRY_AWAIT(fileExists(narInfoFileFor(storePath))); } catch (...) { co_return result::current_exception(); } diff --git a/lix/libstore/binary-cache-store.hh b/lix/libstore/binary-cache-store.hh index 9f05f9da6..79cb737fc 100644 --- a/lix/libstore/binary-cache-store.hh +++ b/lix/libstore/binary-cache-store.hh @@ -73,7 +73,7 @@ public: BinaryCacheStoreConfig & config() override = 0; const BinaryCacheStoreConfig & config() const override = 0; - virtual bool fileExists(const std::string & path) = 0; + virtual kj::Promise> fileExists(const std::string & path) = 0; virtual void upsertFile(const std::string & path, std::shared_ptr> istream, diff --git a/lix/libstore/http-binary-cache-store.cc b/lix/libstore/http-binary-cache-store.cc index 5050a7f49..9dd1f6a19 100644 --- a/lix/libstore/http-binary-cache-store.cc +++ b/lix/libstore/http-binary-cache-store.cc @@ -120,16 +120,18 @@ protected: throw SubstituterDisabled("substituter '%s' is disabled", getUri()); } - bool fileExists(const std::string & path) override - { + kj::Promise> fileExists(const std::string & path) override + try { checkEnabled(); try { - return getFileTransfer()->exists(makeURI(path)); + co_return getFileTransfer()->exists(makeURI(path)); } catch (FileTransferError & e) { maybeDisable(); throw; } + } catch (...) { + co_return result::current_exception(); } void upsertFile(const std::string & path, diff --git a/lix/libstore/local-binary-cache-store.cc b/lix/libstore/local-binary-cache-store.cc index ce29b3dd9..d6657ef49 100644 --- a/lix/libstore/local-binary-cache-store.cc +++ b/lix/libstore/local-binary-cache-store.cc @@ -57,7 +57,7 @@ public: protected: - bool fileExists(const std::string & path) override; + kj::Promise> fileExists(const std::string & path) override; void upsertFile(const std::string & path, std::shared_ptr> istream, @@ -125,9 +125,11 @@ try { co_return result::current_exception(); } -bool LocalBinaryCacheStore::fileExists(const std::string & path) -{ - return pathExists(binaryCacheDir + "/" + path); +kj::Promise> LocalBinaryCacheStore::fileExists(const std::string & path) +try { + return {pathExists(binaryCacheDir + "/" + path)}; +} catch (...) { + return {result::current_exception()}; } std::set LocalBinaryCacheStore::uriSchemes() diff --git a/lix/libstore/s3-binary-cache-store.cc b/lix/libstore/s3-binary-cache-store.cc index 5799b389d..cbafc23f3 100644 --- a/lix/libstore/s3-binary-cache-store.cc +++ b/lix/libstore/s3-binary-cache-store.cc @@ -324,8 +324,8 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore co_return result::current_exception(); } - bool fileExists(const std::string & path) override - { + kj::Promise> fileExists(const std::string & path) override + try { stats.head++; auto res = s3Helper.client->HeadObject( @@ -339,11 +339,13 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore || error.GetErrorType() == Aws::S3::S3Errors::NO_SUCH_KEY // If bucket listing is disabled, 404s turn into 403s || error.GetErrorType() == Aws::S3::S3Errors::ACCESS_DENIED) - return false; + return {false}; throw Error("AWS error fetching '%s': %s", path, error.GetMessage()); } - return true; + return {true}; + } catch (...) { + return {result::current_exception()}; } std::shared_ptr transferManager;