From 66adbdfd9743cccec5f7ca4992cf3a631bb22774 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 10 Aug 2016 16:06:33 +0200 Subject: [PATCH] HttpBinaryCacheStore: Retry on transient HTTP errors This makes us more robust against 500 errors from CloudFront or S3 (assuming the 500 error isn't cached by CloudFront...). --- src/libstore/download.cc | 35 ++++++++++++++++++------- src/libstore/download.hh | 11 ++++---- src/libstore/http-binary-cache-store.cc | 2 ++ 3 files changed, 34 insertions(+), 14 deletions(-) diff --git a/src/libstore/download.cc b/src/libstore/download.cc index ce5584188..95c7d2255 100644 --- a/src/libstore/download.cc +++ b/src/libstore/download.cc @@ -8,6 +8,7 @@ #include #include +#include namespace nix { @@ -194,7 +195,11 @@ struct CurlDownloader : public Downloader if (res != CURLE_OK) { Error err = httpStatus == 404 ? NotFound : - httpStatus == 403 ? Forbidden : Misc; + httpStatus == 403 ? Forbidden : + (httpStatus == 408 || httpStatus == 500 || httpStatus == 503 + || httpStatus == 504 || httpStatus == 522 || httpStatus == 524 + || res == CURLE_COULDNT_RESOLVE_HOST) ? Transient : + Misc; if (res == CURLE_HTTP_RETURNED_ERROR && httpStatus != -1) throw DownloadError(err, format("unable to download ā€˜%sā€™: HTTP error %d") % url % httpStatus); @@ -210,14 +215,26 @@ struct CurlDownloader : public Downloader DownloadResult download(string url, const DownloadOptions & options) override { - DownloadResult res; - if (fetch(resolveUri(url), options)) { - res.cached = false; - res.data = data; - } else - res.cached = true; - res.etag = etag; - return res; + size_t attempt = 0; + + while (true) { + try { + DownloadResult res; + if (fetch(resolveUri(url), options)) { + res.cached = false; + res.data = data; + } else + res.cached = true; + res.etag = etag; + return res; + } catch (DownloadError & e) { + attempt++; + if (e.error != Transient || attempt >= options.tries) throw; + auto ms = 25 * (1 << (attempt - 1)); + printMsg(lvlError, format("warning: %s; retrying in %d ms") % e.what() % ms); + std::this_thread::sleep_for(std::chrono::milliseconds(ms)); + } + } } }; diff --git a/src/libstore/download.hh b/src/libstore/download.hh index efddc5528..1f6098759 100644 --- a/src/libstore/download.hh +++ b/src/libstore/download.hh @@ -9,10 +9,11 @@ namespace nix { struct DownloadOptions { - string expectedETag; - bool verifyTLS{true}; - enum { yes, no, automatic } showProgress{yes}; - bool head{false}; + std::string expectedETag; + bool verifyTLS = true; + enum { yes, no, automatic } showProgress = yes; + bool head = false; + size_t tries = 1; }; struct DownloadResult @@ -31,7 +32,7 @@ struct Downloader Path downloadCached(ref store, const string & url, bool unpack, const Hash & expectedHash = Hash()); - enum Error { NotFound, Forbidden, Misc }; + enum Error { NotFound, Forbidden, Misc, Transient }; }; ref makeDownloader(); diff --git a/src/libstore/http-binary-cache-store.cc b/src/libstore/http-binary-cache-store.cc index da80b636c..42e9099b8 100644 --- a/src/libstore/http-binary-cache-store.cc +++ b/src/libstore/http-binary-cache-store.cc @@ -58,6 +58,7 @@ protected: DownloadOptions options; options.showProgress = DownloadOptions::no; options.head = true; + options.tries = 5; downloader->download(cacheUri + "/" + path, options); return true; } catch (DownloadError & e) { @@ -79,6 +80,7 @@ protected: auto downloader(downloaders.get()); DownloadOptions options; options.showProgress = DownloadOptions::no; + options.tries = 3; try { return downloader->download(cacheUri + "/" + path, options).data; } catch (DownloadError & e) {