From 2b76094a237cee253ebfc89bab448a5ff2e503b8 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <eelco.dolstra@logicblox.com>
Date: Fri, 19 Feb 2016 17:41:11 +0100
Subject: [PATCH] S3BinaryCacheStore::isValidPath(): Do a GET instead of HEAD

---
 src/hydra-queue-runner/binary-cache-store.cc  |  4 +--
 src/hydra-queue-runner/binary-cache-store.hh  |  2 ++
 .../s3-binary-cache-store.cc                  | 28 ++++++++++++++++++-
 .../s3-binary-cache-store.hh                  |  2 ++
 4 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/src/hydra-queue-runner/binary-cache-store.cc b/src/hydra-queue-runner/binary-cache-store.cc
index 226c8dda..6370c3b4 100644
--- a/src/hydra-queue-runner/binary-cache-store.cc
+++ b/src/hydra-queue-runner/binary-cache-store.cc
@@ -103,12 +103,12 @@ NarInfo BinaryCacheStore::readNarInfo(const Path & storePath)
         }
     }
 
-    stats.narInfoRead++;
-
     auto narInfoFile = narInfoFileFor(storePath);
     auto narInfo = make_ref<NarInfo>(getFile(narInfoFile), narInfoFile);
     assert(narInfo->path == storePath);
 
+    stats.narInfoRead++;
+
     if (publicKeys) {
         if (!narInfo->checkSignature(*publicKeys))
             throw Error(format("invalid signature on NAR info file ‘%1%’") % narInfoFile);
diff --git a/src/hydra-queue-runner/binary-cache-store.hh b/src/hydra-queue-runner/binary-cache-store.hh
index 709a18cc..8b2d4a81 100644
--- a/src/hydra-queue-runner/binary-cache-store.hh
+++ b/src/hydra-queue-runner/binary-cache-store.hh
@@ -74,6 +74,8 @@ private:
 
     void addToCache(const ValidPathInfo & info, const string & nar);
 
+protected:
+
     NarInfo readNarInfo(const Path & storePath);
 
 public:
diff --git a/src/hydra-queue-runner/s3-binary-cache-store.cc b/src/hydra-queue-runner/s3-binary-cache-store.cc
index cec6f9cf..3f6b6bf9 100644
--- a/src/hydra-queue-runner/s3-binary-cache-store.cc
+++ b/src/hydra-queue-runner/s3-binary-cache-store.cc
@@ -1,5 +1,7 @@
 #include "s3-binary-cache-store.hh"
 
+#include "nar-info.hh"
+
 #include <aws/core/client/ClientConfiguration.h>
 #include <aws/s3/S3Client.h>
 #include <aws/s3/model/CreateBucketRequest.h>
@@ -10,13 +12,22 @@
 
 namespace nix {
 
+struct S3Error : public Error
+{
+    Aws::S3::S3Errors err;
+    S3Error(Aws::S3::S3Errors err, const FormatOrString & fs)
+        : Error(fs), err(err) { };
+};
+
 /* Helper: given an Outcome<R, E>, return R in case of success, or
    throw an exception in case of an error. */
 template<typename R, typename E>
 R && checkAws(Aws::Utils::Outcome<R, E> && outcome)
 {
     if (!outcome.IsSuccess())
-        throw Error(format("AWS error: %1%") % outcome.GetError().GetMessage());
+        throw S3Error(
+            outcome.GetError().GetErrorType(),
+            format("AWS error: %1%") % outcome.GetError().GetMessage());
     return outcome.GetResultWithOwnership();
 }
 
@@ -67,6 +78,21 @@ const S3BinaryCacheStore::Stats & S3BinaryCacheStore::getS3Stats()
     return stats;
 }
 
+/* This is a specialisation of isValidPath() that optimistically
+   fetches the .narinfo file, rather than first checking for its
+   existence via a HEAD request. Since .narinfos are small, doing a
+   GET is unlikely to be slower than HEAD. */
+bool S3BinaryCacheStore::isValidPath(const Path & storePath)
+{
+    try {
+        readNarInfo(storePath);
+        return true;
+    } catch (S3Error & e) {
+        if (e.err == Aws::S3::S3Errors::NO_SUCH_KEY) return false;
+        throw;
+    }
+}
+
 bool S3BinaryCacheStore::fileExists(const std::string & path)
 {
     stats.head++;
diff --git a/src/hydra-queue-runner/s3-binary-cache-store.hh b/src/hydra-queue-runner/s3-binary-cache-store.hh
index 15d717c9..a0dd7813 100644
--- a/src/hydra-queue-runner/s3-binary-cache-store.hh
+++ b/src/hydra-queue-runner/s3-binary-cache-store.hh
@@ -39,6 +39,8 @@ public:
 
     const Stats & getS3Stats();
 
+    bool isValidPath(const Path & storePath) override;
+
 private:
 
     Stats stats;