Narinfo negative and transient error caching

Narinfo negative and transient error caching

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 1304 bytes --]

Hey,

This has been on my mind for a while, as I wonder what effect it has on
users fetching substitues.

The narinfo caching as I understand it works as follows:

 Default success TTL => 36 hours
 Negative TTL        => 1 hour
 Transient error TTL => 10 minutes

I'm ignoring the success TTL, I'm just interested in the negative and
transient error values. Negative means that when a server says it
doesn't have an output, that response will be cached for an
hour. Transient errors are for other HTTP response codes, like 504.

I had a look through the Git history, caching negative lookups has been
a thing for a while. Caching transient errors was added, but I couldn't
see why.

Personally I don't see a reason to keep either behaviours?

In an extreme case, the Guix Build Coordinator has to work hard to work
around this caching. Asking the guix-daemon if a substitute exists is
dangerous, as it literally costs an hour if that substitute isn't
available yet, but will be shortly (which happens all the time when
building a bunch of things). Currently it checks itself, and only
continues to ask the guix-daemon to fetch the item if it knows it to
exist. The transient error caching is also problematic, as that imposes
a 10 minute penalty if there's a server issue.

Any thoughts?

Thanks,

Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

Re: Narinfo negative and transient error caching

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 903 bytes --]


Christopher Baines <mail@cbaines.net> writes:

> This has been on my mind for a while, as I wonder what effect it has on
> users fetching substitues.
>
> The narinfo caching as I understand it works as follows:
>
>  Default success TTL => 36 hours
>  Negative TTL        => 1 hour
>  Transient error TTL => 10 minutes
>
> I'm ignoring the success TTL, I'm just interested in the negative and
> transient error values. Negative means that when a server says it
> doesn't have an output, that response will be cached for an
> hour. Transient errors are for other HTTP response codes, like 504.
>
> I had a look through the Git history, caching negative lookups has been
> a thing for a while. Caching transient errors was added, but I couldn't
> see why.
>
> Personally I don't see a reason to keep either behaviours?

I've now sent a patch to remove this behaviour:

  https://issues.guix.gnu.org/47897

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

Re: Narinfo negative and transient error caching

[Ludovic Courtès]

Hi!

(“Sorry for the long delay” is officially my motto at this point.)

Christopher Baines <mail@cbaines.net> skribis:

> This has been on my mind for a while, as I wonder what effect it has on
> users fetching substitues.
>
> The narinfo caching as I understand it works as follows:
>
>  Default success TTL => 36 hours
>  Negative TTL        => 1 hour
>  Transient error TTL => 10 minutes
>
> I'm ignoring the success TTL, I'm just interested in the negative and
> transient error values. Negative means that when a server says it
> doesn't have an output, that response will be cached for an
> hour. Transient errors are for other HTTP response codes, like 504.

You’re looking at the default TTLs, which are not the actual TTLs.
Specifically, servers can include a ‘Cache-Control’ header in their
reply specifying the TTL of their choice, and ‘guix substitute’ honors
that:

  https://git.savannah.gnu.org/cgit/guix.git/tree/guix/substitutes.scm#n200
  https://git.savannah.gnu.org/cgit/guix.git/tree/guix/scripts/publish.scm#n371

‘guix publish’ returns 404 with a TTL of 5mn when the requested item is
in store but needs to be “baked”.

However, ‘guix publish’ does not set ‘Cache-Control’ when the request
item is not in store.  In that case, clients use ‘%narinfo-negative-ttl’
(1h).

> I had a look through the Git history, caching negative lookups has been
> a thing for a while. Caching transient errors was added, but I couldn't
> see why.

Transient error caching was most likely added in the days of
hydra.gnu.org, that VM that was extremely slow.  When overloaded, you’d
get 500 or similar, and at that point it was safer for clients to wait
and come back later, possibly much later.  :-)

> Personally I don't see a reason to keep either behaviours?

The main arguments for these negative TTLs are:

  1. Reducing server load: if the server doesn’t have libreoffice, don’t
     come back asking every 10s, it’s prolly useless.  You could easily
     have “GET storms” for libreoffice if clients don’t restrain
     themselves.

  2. Improving client performance: don’t GET things that are likely to
     fail.

Now, the penalty it imposes is annoying.  I’ve sometimes found myself
working around it, too (because I knew the server was going to have the
store item sooner than 1h).

Rather than removing it entirely, I can think of these options:

  1. Reduce the default negative timeouts.

  2. Add an option to ‘guix publish’ (and to the Coordinator?) so they
     send a ‘Cache-Control’ header with the chosen TTL on 404.  That
     way, if the server operator doesn’t mind extra load, they can run
     “guix publish --negative-ttl=0”.

WDYT?  Does that make any sense?

Ludo’.

Re: Narinfo negative and transient error caching

[Ludovic Courtès]

BTW, one thing that would be interesting too is to return 404 with a
long ‘Cache-Control’ validity when the requested store item is among the
cached failures.

We could also add an extra response header to explicitly communicate
that the store item is known to fail to build.

Ludo’.

Re: Narinfo negative and transient error caching

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 4400 bytes --]


Ludovic Courtès <ludo@gnu.org> writes:

> Hi!
>
> (“Sorry for the long delay” is officially my motto at this point.)
>
> Christopher Baines <mail@cbaines.net> skribis:
>
>> This has been on my mind for a while, as I wonder what effect it has on
>> users fetching substitues.
>>
>> The narinfo caching as I understand it works as follows:
>>
>>  Default success TTL => 36 hours
>>  Negative TTL        => 1 hour
>>  Transient error TTL => 10 minutes
>>
>> I'm ignoring the success TTL, I'm just interested in the negative and
>> transient error values. Negative means that when a server says it
>> doesn't have an output, that response will be cached for an
>> hour. Transient errors are for other HTTP response codes, like 504.
>
> You’re looking at the default TTLs, which are not the actual TTLs.
> Specifically, servers can include a ‘Cache-Control’ header in their
> reply specifying the TTL of their choice, and ‘guix substitute’ honors
> that:
>
>   https://git.savannah.gnu.org/cgit/guix.git/tree/guix/substitutes.scm#n200
>   https://git.savannah.gnu.org/cgit/guix.git/tree/guix/scripts/publish.scm#n371
>
> ‘guix publish’ returns 404 with a TTL of 5mn when the requested item is
> in store but needs to be “baked”.
>
> However, ‘guix publish’ does not set ‘Cache-Control’ when the request
> item is not in store.  In that case, clients use ‘%narinfo-negative-ttl’
> (1h).

You're right that the negative ttl is just a default, so it's possible
to override the default behaviour in the success and negative lookup
cases, but I don't believe the Cache-Control header is used for
transient errors.

>> I had a look through the Git history, caching negative lookups has been
>> a thing for a while. Caching transient errors was added, but I couldn't
>> see why.
>
> Transient error caching was most likely added in the days of
> hydra.gnu.org, that VM that was extremely slow.  When overloaded, you’d
> get 500 or similar, and at that point it was safer for clients to wait
> and come back later, possibly much later.  :-)
>
>> Personally I don't see a reason to keep either behaviours?
>
> The main arguments for these negative TTLs are:
>
>   1. Reducing server load: if the server doesn’t have libreoffice, don’t
>      come back asking every 10s, it’s prolly useless.  You could easily
>      have “GET storms” for libreoffice if clients don’t restrain
>      themselves.
>
>   2. Improving client performance: don’t GET things that are likely to
>      fail.

As you say, for the negative TTL, the question here is really what's the
best default value, if a server isn't specifying one.

Given that most narinfo requests precede a build for that thing if the
response is negative, I have my doubts about those two arguments
above. This is assuming the most common case is users asking guix to
install and upgrade things.

If a user gets a negative response, they'll just build it instead and
not check for that narinfo again. Even if they cancel that build when
they realise they don't want to build libreoffice, they'll wait a bit
anyway before retrying.

> Now, the penalty it imposes is annoying.  I’ve sometimes found myself
> working around it, too (because I knew the server was going to have the
> store item sooner than 1h).
>
> Rather than removing it entirely, I can think of these options:
>
>   1. Reduce the default negative timeouts.

I think reducing it is good, as you say, it's possible to override the
default from the server side. Just in case someone wants caching
behaviour, it might be worth keeping that functionality at least.

>   2. Add an option to ‘guix publish’ (and to the Coordinator?) so they
>      send a ‘Cache-Control’ header with the chosen TTL on 404.  That
>      way, if the server operator doesn’t mind extra load, they can run
>      “guix publish --negative-ttl=0”.

That sounds sensible. The Guix Build Coordinator doesn't do any serving,
that's left to something else like nginx. For the deployments I maintain
though, I don't think I'm setting the relevant headers, but I'll look at
changing that.

Going back to the %narinfo-transient-error-ttl, if I'm correct in saying
that it's not possible to override that, maybe that should also use the
relevant header value if set?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

[bug#47897] [PATCH 1/2] publish: Add '--negative-ttl'.

[Ludovic Courtès]

* guix/scripts/publish.scm (show-help, %options): Add '--negative-ttl'.
(render-narinfo, render-narinfo/cached, make-request-handler): Add #:negative-ttl
and honor it.
(run-publish-server): Add #:narinfo-negative-ttl and honor it.
(guix-publish): Honor '--negative-ttl'.
* tests/publish.scm ("negative TTL", "no negative TTL"): New tests.
---
 doc/guix.texi            | 10 ++++++++++
 guix/scripts/publish.scm | 30 ++++++++++++++++++++++--------
 tests/publish.scm        | 32 +++++++++++++++++++++++++++++++-
 3 files changed, 63 insertions(+), 9 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 0947b9f028..a34b2fca1e 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -12727,6 +12727,16 @@ Additionally, when @option{--cache} is used, cached entries that have
 not been accessed for @var{ttl} and that no longer have a corresponding
 item in the store, may be deleted.
 
+@item --negative-ttl=@var{ttl}
+Similarly produce @code{Cache-Control} HTTP headers to advertise the
+time-to-live (TTL) of @emph{negative} lookups---missing store items, for
+which the HTTP 404 code is returned.  By default, no negative TTL is
+advertised.
+
+This parameter can help adjust server load and substitute latency by
+instructing cooperating clients to be more or less patient when a store
+item is missing.
+
 @item --cache-bypass-threshold=@var{size}
 When used in conjunction with @option{--cache}, store items smaller than
 @var{size} are immediately available, even when they are not yet in
diff --git a/guix/scripts/publish.scm b/guix/scripts/publish.scm
index 39bb224cad..ef6fa5f074 100644
--- a/guix/scripts/publish.scm
+++ b/guix/scripts/publish.scm
@@ -1,7 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2015 David Thompson <davet@gnu.org>
 ;;; Copyright © 2020 by Amar M. Singh <nly@disroot.org>
-;;; Copyright © 2015, 2016, 2017, 2018, 2019, 2020 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2015, 2016, 2017, 2018, 2019, 2020, 2021 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2020 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
 ;;;
@@ -101,6 +101,8 @@ Publish ~a over HTTP.\n") %store-directory)
       --workers=N        use N workers to bake items"))
   (display (G_ "
       --ttl=TTL          announce narinfos can be cached for TTL seconds"))
+  (display (G_ "
+      --negative-ttl=TTL announce missing narinfos can be cached for TTL seconds"))
   (display (G_ "
       --nar-path=PATH    use PATH as the prefix for nar URLs"))
   (display (G_ "
@@ -224,6 +226,13 @@ usage."
                       (leave (G_ "~a: invalid duration~%") arg))
                     (alist-cons 'narinfo-ttl (time-second duration)
                                 result))))
+        (option '("negative-ttl") #t #f
+                (lambda (opt name arg result)
+                  (let ((duration (string->duration arg)))
+                    (unless duration
+                      (leave (G_ "~a: invalid duration~%") arg))
+                    (alist-cons 'narinfo-negative-ttl (time-second duration)
+                                result))))
         (option '("nar-path") #t #f
                 (lambda (opt name arg result)
                   (alist-cons 'nar-path arg result)))
@@ -390,14 +399,14 @@ References: ~a~%"
 
 (define* (render-narinfo store request hash
                          #:key ttl (compressions (list %no-compression))
-                         (nar-path "nar"))
+                         (nar-path "nar") negative-ttl)
   "Render metadata for the store path corresponding to HASH.  If TTL is true,
 advertise it as the maximum validity period (in seconds) via the
 'Cache-Control' header.  This allows 'guix substitute' to cache it for an
 appropriate duration.  NAR-PATH specifies the prefix for nar URLs."
   (let ((store-path (hash-part->path store hash)))
     (if (string-null? store-path)
-        (not-found request #:phrase "")
+        (not-found request #:phrase "" #:ttl negative-ttl)
         (values `((content-type . (application/x-nix-narinfo))
                   ,@(if ttl
                         `((cache-control (max-age . ,ttl)))
@@ -512,7 +521,7 @@ interpreted as the basename of a store item."
 
 (define* (render-narinfo/cached store request hash
                                 #:key ttl (compressions (list %no-compression))
-                                (nar-path "nar")
+                                (nar-path "nar") negative-ttl
                                 cache pool)
   "Respond to the narinfo request for REQUEST.  If the narinfo is available in
 CACHE, then send it; otherwise, return 404 and \"bake\" that nar and narinfo
@@ -536,7 +545,7 @@ requested using POOL."
                                                 #:compression
                                                 (first compressions)))))
     (cond ((string-null? item)
-           (not-found request))
+           (not-found request #:ttl negative-ttl))
           ((file-exists? cached)
            ;; Narinfo is in cache, send it.
            (values `((content-type . (application/x-nix-narinfo))
@@ -584,7 +593,7 @@ requested using POOL."
                           #:phrase "We're baking it"
                           #:ttl 300)))          ;should be available within 5m
           (else
-           (not-found request #:phrase "")))))
+           (not-found request #:phrase "" #:ttl negative-ttl)))))
 
 (define (compress-nar cache item compression)
   "Save in directory CACHE the nar for ITEM compressed with COMPRESSION."
@@ -974,7 +983,7 @@ methods, return the applicable compression."
 (define* (make-request-handler store
                                #:key
                                cache pool
-                               narinfo-ttl
+                               narinfo-ttl narinfo-negative-ttl
                                (nar-path "nar")
                                (compressions (list %no-compression)))
   (define compression-type?
@@ -1006,10 +1015,12 @@ methods, return the applicable compression."
                                       #:cache cache
                                       #:pool pool
                                       #:ttl narinfo-ttl
+                                      #:negative-ttl narinfo-negative-ttl
                                       #:nar-path nar-path
                                       #:compressions compressions)
                (render-narinfo store request hash
                                #:ttl narinfo-ttl
+                               #:negative-ttl narinfo-negative-ttl
                                #:nar-path nar-path
                                #:compressions compressions)))
           ;; /nar/file/NAME/sha256/HASH
@@ -1068,7 +1079,7 @@ methods, return the applicable compression."
                              #:key
                              advertise? port
                              (compressions (list %no-compression))
-                             (nar-path "nar") narinfo-ttl
+                             (nar-path "nar") narinfo-ttl narinfo-negative-ttl
                              cache pool)
   (when advertise?
     (let ((name (service-name)))
@@ -1084,6 +1095,7 @@ methods, return the applicable compression."
                                     #:pool pool
                                     #:nar-path nar-path
                                     #:narinfo-ttl narinfo-ttl
+                                    #:narinfo-negative-ttl narinfo-negative-ttl
                                     #:compressions compressions)
               concurrent-http-server
               `(#:socket ,socket)))
@@ -1127,6 +1139,7 @@ methods, return the applicable compression."
            (user        (assoc-ref opts 'user))
            (port        (assoc-ref opts 'port))
            (ttl         (assoc-ref opts 'narinfo-ttl))
+           (negative-ttl (assoc-ref opts 'narinfo-negative-ttl))
            (compressions (match (filter-map (match-lambda
                                               (('compression . compression)
                                                compression)
@@ -1192,6 +1205,7 @@ consider using the '--user' option!~%")))
                                                            "publish worker"))
                               #:nar-path nar-path
                               #:compressions compressions
+                              #:narinfo-negative-ttl negative-ttl
                               #:narinfo-ttl ttl))))))
 
 ;;; Local Variables:
diff --git a/tests/publish.scm b/tests/publish.scm
index 3e67c435ac..c3d086995a 100644
--- a/tests/publish.scm
+++ b/tests/publish.scm
@@ -1,7 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2015 David Thompson <davet@gnu.org>
 ;;; Copyright © 2020 by Amar M. Singh <nly@disroot.org>
-;;; Copyright © 2016, 2017, 2018, 2019, 2020 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2016, 2017, 2018, 2019, 2020, 2021 Ludovic Courtès <ludo@gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -700,6 +700,36 @@ References: ~%"
             (= (response-content-length response) (stat:size (stat log)))
             (first (response-content-type response))))))
 
+(test-equal "negative TTL"
+  `(404 42)
+
+  (call-with-temporary-directory
+   (lambda (cache)
+     (let ((thread (with-separate-output-ports
+                    (call-with-new-thread
+                     (lambda ()
+                       (guix-publish "--port=6786" "-C0"
+                                     "--negative-ttl=42s"))))))
+       (wait-until-ready 6786)
+
+       (let* ((base     "http://localhost:6786/")
+              (url      (string-append base (make-string 32 #\z)
+                                       ".narinfo"))
+              (response (http-get url)))
+         (list (response-code response)
+               (match (assq-ref (response-headers response) 'cache-control)
+                 ((('max-age . ttl)) ttl)
+                 (_ #f))))))))
+
+(test-equal "no negative TTL"
+  `(404 #f)
+  (let* ((uri      (publish-uri
+                    (string-append "/" (make-string 32 #\z)
+                                   ".narinfo")))
+         (response (http-get uri)))
+    (list (response-code response)
+          (assq-ref (response-headers response) 'cache-control))))
+
 (test-equal "/log/NAME not found"
   404
   (let ((uri (publish-uri "/log/does-not-exist")))
-- 
2.31.1

[bug#47897] [PATCH 2/2] substitutes: Reduce negative TTLs.

[Ludovic Courtès]

* guix/substitutes.scm (%narinfo-negative-ttl): Change to 15mn.
(%narinfo-transient-error-ttl): Halve.
---
 guix/substitutes.scm | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/guix/substitutes.scm b/guix/substitutes.scm
index 08f8c24efd..c1b036e42c 100644
--- a/guix/substitutes.scm
+++ b/guix/substitutes.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020, 2021 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2014 Nikita Karetnikov <nikita@karetnikov.org>
 ;;; Copyright © 2018 Kyle Meyer <kyle@kyleam.com>
 ;;; Copyright © 2020 Christopher Baines <mail@cbaines.net>
@@ -72,11 +72,11 @@
 
 (define %narinfo-negative-ttl
   ;; Likewise, but for negative lookups---i.e., cached lookup failures (404).
-  (* 1 3600))
+  (* 15 60))
 
 (define %narinfo-transient-error-ttl
   ;; Likewise, but for transient errors such as 504 ("Gateway timeout").
-  (* 10 60))
+  (* 5 60))
 
 (define %narinfo-cache-directory
   ;; A local cache of narinfos, to avoid going to the network.  Most of the
-- 
2.31.1

Re: bug#47897: [PATCH] substitutes: Don't cache negative lookups or transient errors.

[Ludovic Courtès]

Hi,

Christopher Baines <mail@cbaines.net> skribis:

>> Now, the penalty it imposes is annoying.  I’ve sometimes found myself
>> working around it, too (because I knew the server was going to have the
>> store item sooner than 1h).
>>
>> Rather than removing it entirely, I can think of these options:
>>
>>   1. Reduce the default negative timeouts.
>
> I think reducing it is good, as you say, it's possible to override the
> default from the server side. Just in case someone wants caching
> behaviour, it might be worth keeping that functionality at least.

OK, let’s do that.

>>   2. Add an option to ‘guix publish’ (and to the Coordinator?) so they
>>      send a ‘Cache-Control’ header with the chosen TTL on 404.  That
>>      way, if the server operator doesn’t mind extra load, they can run
>>      “guix publish --negative-ttl=0”.
>
> That sounds sensible. The Guix Build Coordinator doesn't do any serving,
> that's left to something else like nginx. For the deployments I maintain
> though, I don't think I'm setting the relevant headers, but I'll look at
> changing that.

Cool.

> Going back to the %narinfo-transient-error-ttl, if I'm correct in saying
> that it's not possible to override that, maybe that should also use the
> relevant header value if set?

Correct, ‘%narinfo-transient-error-ttl’ cannot be overridden.  We can
halve it if you think that’s useful, thought when that happens, it means
something’s wrong with the server (returning 500 or similar).

I’ve sent patches to address this, lemme know what you think!

Thanks,
Ludo’.

Re: bug#47897: [PATCH] substitutes: Don't cache negative lookups or transient errors.

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 1742 bytes --]


Ludovic Courtès <ludo@gnu.org> writes:

> Hi,
>
> Christopher Baines <mail@cbaines.net> skribis:
>
>>> Now, the penalty it imposes is annoying.  I’ve sometimes found myself
>>> working around it, too (because I knew the server was going to have the
>>> store item sooner than 1h).
>>>
>>> Rather than removing it entirely, I can think of these options:
>>>
>>>   1. Reduce the default negative timeouts.
>>
>> I think reducing it is good, as you say, it's possible to override the
>> default from the server side. Just in case someone wants caching
>> behaviour, it might be worth keeping that functionality at least.
>
> OK, let’s do that.
>
>>>   2. Add an option to ‘guix publish’ (and to the Coordinator?) so they
>>>      send a ‘Cache-Control’ header with the chosen TTL on 404.  That
>>>      way, if the server operator doesn’t mind extra load, they can run
>>>      “guix publish --negative-ttl=0”.
>>
>> That sounds sensible. The Guix Build Coordinator doesn't do any serving,
>> that's left to something else like nginx. For the deployments I maintain
>> though, I don't think I'm setting the relevant headers, but I'll look at
>> changing that.
>
> Cool.
>
>> Going back to the %narinfo-transient-error-ttl, if I'm correct in saying
>> that it's not possible to override that, maybe that should also use the
>> relevant header value if set?
>
> Correct, ‘%narinfo-transient-error-ttl’ cannot be overridden.  We can
> halve it if you think that’s useful, thought when that happens, it means
> something’s wrong with the server (returning 500 or similar).
>
> I’ve sent patches to address this, lemme know what you think!

The patches you've sent look good.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

Re: bug#47897: [PATCH] substitutes: Don't cache negative lookups or transient errors.

[Ludovic Courtès]

Hi,

Christopher Baines <mail@cbaines.net> skribis:

> The patches you've sent look good.

Pushed as 938ffcbb0589adc07dc12c79eda3e1e2bb9e7cf8 (I was generous and
lowered ‘%narinfo-negative-ttl’ to 10mn :-)).

Thanks,
Ludo’.