From a987d92bd0162cadb36b76f8dfd5ac2cbddc97fb Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Tue, 11 Feb 2025 01:06:15 +0100 Subject: [PATCH] libstore: asyncify LocalStore::autoGC sadly we need both a synchronous and an asynchronous promise for this since destructors cannot be async. we also cannot use forked promises since multiple threads may be waiting for an auto-gc to complete, but forked promises are bound to an event loop (and thus a single thread) Change-Id: I7b4fdbd229c4a1e01bbf80858e93347024f2e333 --- lix/libstore/build/worker.cc | 2 +- lix/libstore/gc.cc | 29 ++++++++++++++++++++--------- lix/libstore/local-store.cc | 10 +++++----- lix/libstore/local-store.hh | 6 ++++-- 4 files changed, 30 insertions(+), 17 deletions(-) diff --git a/lix/libstore/build/worker.cc b/lix/libstore/build/worker.cc index ff8ba2ab4..f38e6cb6a 100644 --- a/lix/libstore/build/worker.cc +++ b/lix/libstore/build/worker.cc @@ -313,7 +313,7 @@ kj::Promise> Worker::boopGC(LocalStore & localStore) try { while (true) { co_await AIO().provider.getTimer().afterDelay(10 * kj::SECONDS); - localStore.autoGC(false); + TRY_AWAIT(localStore.autoGC(false)); } } catch (...) { co_return result::current_exception(); diff --git a/lix/libstore/gc.cc b/lix/libstore/gc.cc index c8c78b5f4..3e62a2e85 100644 --- a/lix/libstore/gc.cc +++ b/lix/libstore/gc.cc @@ -8,6 +8,7 @@ #include "lix/libutil/strings.hh" #include "lix/libutil/thread-name.hh" +#include #include #include @@ -868,8 +869,8 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results) } -void LocalStore::autoGC(bool sync) -{ +kj::Promise> LocalStore::autoGC(bool sync) +try { static auto fakeFreeSpaceFile = getEnv("_NIX_TEST_FREE_SPACE_FILE"); auto getAvail = [this]() -> uint64_t { @@ -883,33 +884,35 @@ void LocalStore::autoGC(bool sync) return (uint64_t) st.f_bavail * st.f_frsize; }; - std::shared_future future; + auto pfp = kj::newPromiseAndCrossThreadFulfiller(); { auto state(_gcState.lock()); if (state->gcRunning) { - future = state->gcFuture; + state->gcWaiters.push_back(std::move(pfp.fulfiller)); debug("waiting for auto-GC to finish"); goto sync; } auto now = std::chrono::steady_clock::now(); - if (now < state->lastGCCheck + std::chrono::seconds(settings.minFreeCheckInterval)) return; + if (now < state->lastGCCheck + std::chrono::seconds(settings.minFreeCheckInterval)) { + co_return result::success(); + } auto avail = getAvail(); state->lastGCCheck = now; - if (avail >= settings.minFree || avail >= settings.maxFree) return; + if (avail >= settings.minFree || avail >= settings.maxFree) co_return result::success(); - if (avail > state->availAfterGC * 0.97) return; + if (avail > state->availAfterGC * 0.97) co_return result::success(); state->gcRunning = true; std::promise promise; - future = state->gcFuture = promise.get_future().share(); + state->gcFuture = promise.get_future(); std::thread([promise{std::move(promise)}, this, avail, getAvail]() mutable { setCurrentThreadName("auto gc"); @@ -922,6 +925,10 @@ void LocalStore::autoGC(bool sync) state->gcRunning = false; state->lastGCCheck = std::chrono::steady_clock::now(); promise.set_value(); + for (auto & waiter : state->gcWaiters) { + waiter->fulfill(); + } + state->gcWaiters.clear(); }); GCOptions options; @@ -946,7 +953,11 @@ void LocalStore::autoGC(bool sync) sync: // Wait for the future outside of the state lock. - if (sync) future.get(); + if (sync) co_await pfp.promise; + + co_return result::success(); +} catch (...) { + co_return result::current_exception(); } diff --git a/lix/libstore/local-store.cc b/lix/libstore/local-store.cc index 8d802fea5..9e06b7117 100644 --- a/lix/libstore/local-store.cc +++ b/lix/libstore/local-store.cc @@ -458,12 +458,12 @@ AutoCloseFD LocalStore::openGCLock() LocalStore::~LocalStore() { - std::shared_future future; + std::future future; { auto state(_gcState.lock()); if (state->gcRunning) - future = state->gcFuture; + future = std::move(state->gcFuture); } if (future.valid()) { @@ -1266,7 +1266,7 @@ try { } } - autoGC(); + TRY_AWAIT(autoGC()); canonicalisePathMetaData(realPath, {}); @@ -1381,7 +1381,7 @@ try { deletePath(realPath); - autoGC(); + TRY_AWAIT(autoGC()); if (inMemory) { StringSource dumpSource { dump }; @@ -1448,7 +1448,7 @@ try { deletePath(realPath); - autoGC(); + TRY_AWAIT(autoGC()); writeFile(realPath, s); diff --git a/lix/libstore/local-store.hh b/lix/libstore/local-store.hh index 3ce451fbb..66263fe15 100644 --- a/lix/libstore/local-store.hh +++ b/lix/libstore/local-store.hh @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -118,7 +119,8 @@ private: * the GC to finish. */ bool gcRunning = false; - std::shared_future gcFuture; + std::future gcFuture; + std::list>> gcWaiters; /** * How much disk space was available after the previous @@ -297,7 +299,7 @@ public: * If free disk space in /nix/store if below minFree, delete * garbage until it exceeds maxFree. */ - void autoGC(bool sync = true); + kj::Promise> autoGC(bool sync = true); /** * Register the store path 'output' as the output named 'outputName' of