Subject: bug#26201: No notification of cache misses when downloading substitutes

From: Tobias Geerinckx-Rice <[email-masked]>
Date: Tue, 21 Mar 2017 15:55:05 +0100

Hullo!

On 21/03/17 07:49, [email-masked] wrote:
> I'm not sure how any of this matters. If you are running a local 
> Hydra instance or whatever, then I'd assume you'd be aware of what, 
> if any, problems that could arise.

It matters for the reasons mentioned. It's not a ‘local Hydra’ & I have
no idea what problems you're talking about.

My problem is that every invocation of Guix already fills several
screens with Guile cache misses. Adding another warning (‘warning! the
system is working exactly as designed!’) will only serve to make those
other warnings look less silly, and I think that would be a shame.

To clarify:

- Warnings should be scary because warnings should be actionable.
  There's nothing the user can or needs to do about a cache miss.
- It would be randomly shown to everyone, since this happens constantly.
- The behaviour warned about is not incorrect or abnormal.
- As already noted, it's how caching works.

> I don't see how this would have to be "an ugly hack". It's simply a 
> query and response. The simplest way I can see for this to work would
> be for mirror.hydra to either just send the requested file, or a
> response that the file isn't cached then start to trickle the file on
> to the client.

Well, yeah... That's the ugly hack. :-)

It's not that your suggestion's hard to implement. In fact, it's
just one line for nginx (which it turns out I already had):

  add_header X-Cache-Status $upstream_cache_status;

and 6 lines of lightly-tested Guile (attached)¹. And presto. This thing.

Doesn't mean we should.

Kind regards,

T G-R

¹: Why? Practice. Irony. Light masochism.
From 6d459a442d73628a0628385283c7cf04dff1b797 Mon Sep 17 00:00:00 2001
From: Tobias Geerinckx-Rice <[email-masked]>
Date: Tue, 21 Mar 2017 15:31:56 +0100
Subject: [PATCH] http-client: Warn on proxy cache misses.

Still not a good idea.

* guix/http-client.scm (http-fetch): Add #:peek-behind-proxy parameter
to expose caching proxy implementation details as a scary warning.
* guix/scripts/substitute.scm (fetch): Use it.
---
 guix/http-client.scm        | 10 +++++++++-
 guix/scripts/substitute.scm |  3 ++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/guix/http-client.scm b/guix/http-client.scm
index 6874c51..2366f5e 100644
--- a/guix/http-client.scm
+++ b/guix/http-client.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017 Ludovic Courtès <[email-masked]>
 ;;; Copyright © 2015 Mark H Weaver <[email-masked]>
 ;;; Copyright © 2012, 2015 Free Software Foundation, Inc.
+;;; Copyright © 2017 Tobias Geerinckx-Rice <[email-masked]>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -222,7 +223,8 @@ or if EOF is reached."
 
 (define* (http-fetch uri #:key port (text? #f) (buffered? #t)
                      keep-alive? (verify-certificate? #t)
-                     (headers '((user-agent . "GNU Guile"))))
+                     (headers '((user-agent . "GNU Guile")))
+                     (peek-behind-cache? #f))
   "Return an input port containing the data at URI, and the expected number of
 bytes available or #f.  If TEXT? is true, the data at URI is considered to be
 textual.  Follow any HTTP redirection.  When BUFFERED? is #f, return an
@@ -253,8 +255,14 @@ Raise an '&http-get-error' condition if downloading fails."
                      (http-get uri #:streaming? #t #:port port
                                #:keep-alive? #t
                                #:headers headers))
+                    ((headers)
+                     (response-headers resp))
                     ((code)
                      (response-code resp)))
+        (when (and peek-behind-cache?
+                   (equal? (assoc-ref headers 'x-cache-status) "MISS"))
+              (warning (_ "the caching proxy is working properly!~%"))
+              (warning (_ "and there's nothing you can do about it.~%")))
         (case code
           ((200)
            (values data (response-content-length resp)))
diff --git a/guix/scripts/substitute.scm b/guix/scripts/substitute.scm
index faeb019..4a4f115 100755
--- a/guix/scripts/substitute.scm
+++ b/guix/scripts/substitute.scm
@@ -216,7 +216,8 @@ provide."
                (unless (or buffered? (not (file-port? port)))
                  (setvbuf port _IONBF)))
              (http-fetch uri #:text? #f #:port port
-                         #:verify-certificate? #f))))))
+                         #:verify-certificate? #f
+                         #:peek-behind-cache? #t))))))
     (else
      (leave (_ "unsupported substitute URI scheme: ~a~%")
             (uri->string uri)))))
-- 
2.9.3