From bba3f0a308cceb56bad4aa1efe13927360ae463f Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Fri, 7 Sep 2018 17:08:43 +0200
Subject: [PATCH] If fallback is enabled, temporarily disable substituters
 after a failure

Otherwise, we just keep asking the substituter for other .narinfo
files, which can take a very long time due to retries/timeouts.
---
 src/libstore/build.cc                   |  6 ++++
 src/libstore/http-binary-cache-store.cc | 41 ++++++++++++++++++++++++-
 src/libstore/local-store.cc             |  1 +
 src/libstore/store-api.hh               |  1 +
 4 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index 96ca28742..1402bd097 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -3682,6 +3682,12 @@ void SubstitutionGoal::tryNext()
     } catch (InvalidPath &) {
         tryNext();
         return;
+    } catch (SubstituterDisabled &) {
+        if (settings.tryFallback) {
+            tryNext();
+            return;
+        }
+        throw;
     } catch (Error & e) {
         if (settings.tryFallback) {
             printError(e.what());
diff --git a/src/libstore/http-binary-cache-store.cc b/src/libstore/http-binary-cache-store.cc
index ab524d523..8da0e2f9d 100644
--- a/src/libstore/http-binary-cache-store.cc
+++ b/src/libstore/http-binary-cache-store.cc
@@ -13,6 +13,14 @@ private:
 
     Path cacheUri;
 
+    struct State
+    {
+        bool enabled = true;
+        std::chrono::steady_clock::time_point disabledUntil;
+    };
+
+    Sync<State> _state;
+
 public:
 
     HttpBinaryCacheStore(
@@ -46,8 +54,33 @@ public:
 
 protected:
 
+    void maybeDisable()
+    {
+        auto state(_state.lock());
+        if (state->enabled && settings.tryFallback) {
+            int t = 60;
+            printError("disabling binary cache '%s' for %s seconds", getUri(), t);
+            state->enabled = false;
+            state->disabledUntil = std::chrono::steady_clock::now() + std::chrono::seconds(t);
+        }
+    }
+
+    void checkEnabled()
+    {
+        auto state(_state.lock());
+        if (state->enabled) return;
+        if (std::chrono::steady_clock::now() > state->disabledUntil) {
+            state->enabled = true;
+            debug("re-enabling binary cache '%s'", getUri());
+            return;
+        }
+        throw SubstituterDisabled("substituter '%s' is disabled", getUri());
+    }
+
     bool fileExists(const std::string & path) override
     {
+        checkEnabled();
+
         try {
             DownloadRequest request(cacheUri + "/" + path);
             request.head = true;
@@ -59,6 +92,7 @@ protected:
                bucket is unlistable, so treat 403 as 404. */
             if (e.error == Downloader::NotFound || e.error == Downloader::Forbidden)
                 return false;
+            maybeDisable();
             throw;
         }
     }
@@ -86,12 +120,14 @@ protected:
 
     void getFile(const std::string & path, Sink & sink) override
     {
+        checkEnabled();
         auto request(makeRequest(path));
         try {
             getDownloader()->download(std::move(request), sink);
         } catch (DownloadError & e) {
             if (e.error == Downloader::NotFound || e.error == Downloader::Forbidden)
                 throw NoSuchBinaryCacheFile("file '%s' does not exist in binary cache '%s'", path, getUri());
+            maybeDisable();
             throw;
         }
     }
@@ -99,15 +135,18 @@ protected:
     void getFile(const std::string & path,
         Callback<std::shared_ptr<std::string>> callback) override
     {
+        checkEnabled();
+
         auto request(makeRequest(path));
 
         getDownloader()->enqueueDownload(request,
-            {[callback](std::future<DownloadResult> result) {
+            {[callback, this](std::future<DownloadResult> result) {
                 try {
                     callback(result.get().data);
                 } catch (DownloadError & e) {
                     if (e.error == Downloader::NotFound || e.error == Downloader::Forbidden)
                         return callback(std::shared_ptr<std::string>());
+                    maybeDisable();
                     callback.rethrow();
                 } catch (...) {
                     callback.rethrow();
diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index 5e392c237..197b9d789 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -880,6 +880,7 @@ void LocalStore::querySubstitutablePathInfos(const PathSet & paths,
                     narInfo ? narInfo->fileSize : 0,
                     info->narSize};
             } catch (InvalidPath) {
+            } catch (SubstituterDisabled) {
             } catch (Error & e) {
                 if (settings.tryFallback)
                     printError(e.what());
diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh
index 7c5b495a4..099818ed6 100644
--- a/src/libstore/store-api.hh
+++ b/src/libstore/store-api.hh
@@ -23,6 +23,7 @@ MakeError(BuildError, Error) /* denotes a permanent build failure */
 MakeError(InvalidPath, Error)
 MakeError(Unsupported, Error)
 MakeError(SubstituteGone, Error)
+MakeError(SubstituterDisabled, Error)
 
 
 struct BasicDerivation;