From patchwork Fri Aug 13 10:30:30 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Mathieu Othacehe X-Patchwork-Id: 32099 Return-Path: X-Original-To: patchwork@mira.cbaines.net Delivered-To: patchwork@mira.cbaines.net Received: by mira.cbaines.net (Postfix, from userid 113) id 52A9A27BC82; Fri, 13 Aug 2021 11:31:16 +0100 (BST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on mira.cbaines.net X-Spam-Level: X-Spam-Status: No, score=-2.9 required=5.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.2 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mira.cbaines.net (Postfix) with ESMTPS id 813ED27BC78 for ; Fri, 13 Aug 2021 11:31:15 +0100 (BST) Received: from localhost ([::1]:50674 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mEUT4-00017J-In for patchwork@mira.cbaines.net; Fri, 13 Aug 2021 06:31:14 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:43102) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mEUSt-000168-4J for guix-patches@gnu.org; Fri, 13 Aug 2021 06:31:03 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:57230) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mEUSs-0001Vf-Od for guix-patches@gnu.org; Fri, 13 Aug 2021 06:31:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mEUSs-00072z-Lc for guix-patches@gnu.org; Fri, 13 Aug 2021 06:31:02 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#50040] [PATCH 2/2] publish: Remove cache bypass support. Resent-From: Mathieu Othacehe Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Fri, 13 Aug 2021 10:31:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 50040 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: 50040@debbugs.gnu.org Cc: Mathieu Othacehe Received: via spool by 50040-submit@debbugs.gnu.org id=B50040.162885064627053 (code B ref 50040); Fri, 13 Aug 2021 10:31:02 +0000 Received: (at 50040) by debbugs.gnu.org; 13 Aug 2021 10:30:46 +0000 Received: from localhost ([127.0.0.1]:40541 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mEUSb-00072G-Or for submit@debbugs.gnu.org; Fri, 13 Aug 2021 06:30:46 -0400 Received: from eggs.gnu.org ([209.51.188.92]:54804) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mEUSX-00071e-V6 for 50040@debbugs.gnu.org; Fri, 13 Aug 2021 06:30:42 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:51806) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mEUSS-00015y-P2 for 50040@debbugs.gnu.org; Fri, 13 Aug 2021 06:30:36 -0400 Received: from [2a01:e0a:19b:d9a0:f2f7:a404:c3d3:f8b4] (port=44574 helo=localhost.localdomain) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mEUSS-0008GJ-5B; Fri, 13 Aug 2021 06:30:36 -0400 From: Mathieu Othacehe Date: Fri, 13 Aug 2021 12:30:30 +0200 Message-Id: <20210813103030.1017-2-othacehe@gnu.org> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210813103030.1017-1-othacehe@gnu.org> References: <20210813103030.1017-1-othacehe@gnu.org> MIME-Version: 1.0 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+patchwork=mira.cbaines.net@gnu.org Sender: "Guix-patches" X-getmail-retrieved-from-mailbox: Patches Unconditionally render narinfo and nar files during baking. This avoids the need for pre-baking nar with a CI system. It also prevents receiving 404 errors during baking. * guix/scripts/publish.scm (%options): Remove the cache-bypass-threshold option. (show-help): Ditto. (cache-bypass-threshold, bypass-cache?): Remove them. (render-narinfo/cached): Render a temporary narinfo unconditionally during baking. (render-nar/cached): Render a live nar unconditionally during baking. (guix-publish): Remove cache bypass option. * gnu/services/base.scm () [cache-bypass-threshold]: Remove it (guix-publish-shepherd-service): Adapt it. * doc/guix.texi (Base services, Invoking guix publish): Document it. * tests/publish.scm ("with cache", "with cache, lzip + gzip", "with cache, uncompressed"): Adapt them. ("with cache, cache bypass", "with cache, cache bypass, unmapped hash part"): Remove them. --- doc/guix.texi | 28 ------------- gnu/services/base.scm | 11 +---- guix/scripts/publish.scm | 54 ++++++------------------ tests/publish.scm | 88 ++++------------------------------------ 4 files changed, 24 insertions(+), 157 deletions(-) diff --git a/doc/guix.texi b/doc/guix.texi index 78c1c09858..adb2ada4e8 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -12752,13 +12752,6 @@ archive is cached in @var{directory}, subsequent requests succeed and are served directly from the cache, which guarantees that clients get the best possible bandwidth. -That first @code{.narinfo} request nonetheless returns 200, provided the -requested store item is ``small enough'', below the cache bypass -threshold---see @option{--cache-bypass-threshold} below. That way, -clients do not have to wait until the archive is baked. For larger -store items, the first @code{.narinfo} request returns 404, meaning that -clients have to wait until the archive is baked. - The ``baking'' process is performed by worker threads. By default, one thread per CPU core is created, but this can be customized. See @option{--workers} below. @@ -12794,21 +12787,6 @@ 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 -cache. @var{size} is a size in bytes, or it can be suffixed by @code{M} -for megabytes and so on. The default is @code{10M}. - -``Cache bypass'' allows you to reduce the publication delay for clients -at the expense of possibly additional I/O and CPU use on the server -side: depending on the client access patterns, those store items can end -up being baked several times until a copy is available in cache. - -Increasing the threshold may be useful for sites that have few users, or -to guarantee that users get substitutes even for store items that are -not popular. - @item --nar-path=@var{path} Use @var{path} as the prefix for the URLs of ``nar'' files (@pxref{Invoking guix archive, normalized archives}). @@ -15818,12 +15796,6 @@ When it is an integer, this is the number of worker threads used for caching; when @code{#f}, the number of processors is used. @xref{Invoking guix publish, @option{--workers}}, for more information. -@item @code{cache-bypass-threshold} (default: 10 MiB) -When @code{cache} is true, this is the maximum size in bytes of a store -item for which @command{guix publish} may bypass its cache in case of a -cache miss. @xref{Invoking guix publish, -@option{--cache-bypass-threshold}}, for more information. - @item @code{ttl} (default: @code{#f}) When it is an integer, this denotes the @dfn{time-to-live} in seconds of the published archives. @xref{Invoking guix publish, @option{--ttl}}, diff --git a/gnu/services/base.scm b/gnu/services/base.scm index c784d312b1..ac469abf8c 100644 --- a/gnu/services/base.scm +++ b/gnu/services/base.scm @@ -1775,8 +1775,6 @@ proxy of 'guix-daemon'...~%") (default "nar")) (cache guix-publish-configuration-cache ;#f | string (default #f)) - (cache-bypass-threshold guix-publish-configuration-cache-bypass-threshold - (default (* 10 (expt 2 20)))) ;integer (workers guix-publish-configuration-workers ;#f | integer (default #f)) (ttl guix-publish-configuration-ttl ;#f | integer @@ -1815,8 +1813,7 @@ raise a deprecation warning if the 'compression-level' field was used." lst)))) (match-record config - (guix port host nar-path cache workers ttl cache-bypass-threshold - advertise?) + (guix port host nar-path cache workers ttl advertise?) (list (shepherd-service (provision '(guix-publish)) (requirement `(user-processes @@ -1843,11 +1840,7 @@ raise a deprecation warning if the 'compression-level' field was used." "s")) #~()) #$@(if cache - #~((string-append "--cache=" #$cache) - #$(string-append - "--cache-bypass-threshold=" - (number->string - cache-bypass-threshold))) + #~((string-append "--cache=" #$cache)) #~())) ;; Make sure we run in a UTF-8 locale so we can produce diff --git a/guix/scripts/publish.scm b/guix/scripts/publish.scm index 981ef8d267..64f444e901 100644 --- a/guix/scripts/publish.scm +++ b/guix/scripts/publish.scm @@ -97,9 +97,6 @@ Publish ~a over HTTP.\n") %store-directory) compress archives with METHOD at LEVEL")) (display (G_ " -c, --cache=DIRECTORY cache published items to DIRECTORY")) - (display (G_ " - --cache-bypass-threshold=SIZE - serve store items below SIZE even when not cached")) (display (G_ " --workers=N use N workers to bake items")) (display (G_ " @@ -214,10 +211,6 @@ usage." (option '(#\c "cache") #t #f (lambda (opt name arg result) (alist-cons 'cache arg result))) - (option '("cache-bypass-threshold") #t #f - (lambda (opt name arg result) - (alist-cons 'cache-bypass-threshold (size->number arg) - result))) (option '("workers") #t #f (lambda (opt name arg result) (alist-cons 'workers (string->number* arg) @@ -513,18 +506,6 @@ vanished from the store in the meantime." result)) (apply throw args)))))) -(define cache-bypass-threshold - ;; Maximum size of a store item that may be served by the '/cached' handlers - ;; below even when not in cache. - (make-parameter (* 10 (expt 2 20)))) - -(define (bypass-cache? store item) - "Return true if we allow ITEM to be downloaded before it is cached. ITEM is -interpreted as the basename of a store item." - (guard (c ((store-error? c) #f)) - (< (path-info-nar-size (query-path-info store item)) - (cache-bypass-threshold)))) - (define* (render-narinfo/cached store request hash #:key ttl (compressions (list %no-compression)) (nar-path "nar") negative-ttl @@ -584,19 +565,14 @@ requested using POOL." #:delete-entry delete-entry #:cleanup-period ttl)))) - ;; If ITEM passes 'bypass-cache?', render a temporary narinfo right - ;; away, with a short TTL. The narinfo is temporary because it - ;; lacks 'FileSize', for instance, which the cached narinfo will - ;; have. Chances are that the nar will be baked by the time the - ;; client asks for it. - (if (bypass-cache? store item) - (render-narinfo store request hash - #:ttl 300 ;temporary - #:nar-path nar-path - #:compressions compressions) - (not-found request - #:phrase "We're baking it" - #:ttl 300))) ;should be available within 5m + ;; Render a temporary narinfo right away, with a short TTL. The + ;; narinfo is temporary because it lacks 'FileSize', for instance, + ;; which the cached narinfo will have. Chances are that the nar + ;; will be baked by the time the client asks for it. + (render-narinfo store request hash + #:ttl 300 ;temporary + #:nar-path nar-path + #:compressions compressions)) (else (not-found request #:phrase "" #:ttl negative-ttl))))) @@ -740,9 +716,9 @@ return it; otherwise, return 404. When TTL is true, use it as the `((cache-control (max-age . ,ttl))) '()) - ;; XXX: We're not returning the actual contents, deferring - ;; instead to 'http-write'. This is a hack to work around - ;; . + ;; XXX: We're not returning the actual contents, + ;; deferring instead to 'http-write'. This is a hack to + ;; work around . (x-raw-file . ,cached)) #f)) ((let* ((hash (and=> (string-index store-item #\-) @@ -750,8 +726,7 @@ return it; otherwise, return 404. When TTL is true, use it as the (item (and hash (guard (c ((store-error? c) #f)) (hash-part->path store hash))))) - (and item (not (string-null? item)) - (bypass-cache? store item))) + (and item (not (string-null? item)))) ;; Render STORE-ITEM live. We reach this because STORE-ITEM is ;; being baked but clients are already asking for it. Thus, we're ;; duplicating work, but doing so allows us to reduce delays. @@ -1306,10 +1281,7 @@ headers." consider using the '--user' option!~%"))) (parameterize ((%public-key public-key) - (%private-key private-key) - (cache-bypass-threshold - (or (assoc-ref opts 'cache-bypass-threshold) - (cache-bypass-threshold)))) + (%private-key private-key)) (info (G_ "publishing ~a on ~a, port ~d~%") %store-directory (inet-ntop (sockaddr:fam address) (sockaddr:addr address)) diff --git a/tests/publish.scm b/tests/publish.scm index c3d086995a..43871a2a93 100644 --- a/tests/publish.scm +++ b/tests/publish.scm @@ -422,15 +422,14 @@ References: ~%" 200 ;nar/gzip/… #t ;Content-Length #t ;FileSize - 404) ;nar/… + 200) ;nar/… (call-with-temporary-directory (lambda (cache) (let ((thread (with-separate-output-ports (call-with-new-thread (lambda () (guix-publish "--port=6797" "-C2" - (string-append "--cache=" cache) - "--cache-bypass-threshold=0")))))) + (string-append "--cache=" cache))))))) (wait-until-ready 6797) (let* ((base "http://localhost:6797/") (part (store-path-hash-part %item)) @@ -441,9 +440,9 @@ References: ~%" (nar (string-append cache "/gzip/" (basename %item) ".nar")) (response (http-get url))) - (and (= 404 (response-code response)) + (and (= 200 (response-code response)) - ;; We should get an explicitly short TTL for 404 in this case + ;; We should get an explicitly short TTL for 200 in this case ;; because it's going to become 200 shortly. (match (assq-ref (response-headers response) 'cache-control) ((('max-age . ttl)) @@ -477,15 +476,14 @@ References: ~%" (response-code uncompressed))))))))) (test-equal "with cache, lzip + gzip" - '(200 200 404) + '(200 200 200) (call-with-temporary-directory (lambda (cache) (let ((thread (with-separate-output-ports (call-with-new-thread (lambda () (guix-publish "--port=6794" "-Cgzip:2" "-Clzip:2" - (string-append "--cache=" cache) - "--cache-bypass-threshold=0")))))) + (string-append "--cache=" cache))))))) (wait-until-ready 6794) (let* ((base "http://localhost:6794/") (part (store-path-hash-part %item)) @@ -533,15 +531,14 @@ References: ~%" (* 42 3600) ;TTL on nar/… (path-info-nar-size (query-path-info %store item)) ;FileSize - 404) ;nar/gzip/… + 200) ;nar/gzip/… (call-with-temporary-directory (lambda (cache) (let ((thread (with-separate-output-ports (call-with-new-thread (lambda () (guix-publish "--port=6796" "-C2" "--ttl=42h" - (string-append "--cache=" cache) - "--cache-bypass-threshold=0")))))) + (string-append "--cache=" cache))))))) (wait-until-ready 6796) (let* ((base "http://localhost:6796/") (part (store-path-hash-part item)) @@ -551,7 +548,7 @@ References: ~%" (nar (string-append cache "/none/" (basename item) ".nar")) (response (http-get url))) - (and (= 404 (response-code response)) + (and (= 200 (response-code response)) (wait-for-file cached) (let* ((response (http-get url)) @@ -611,73 +608,6 @@ References: ~%" (delete-paths %store (list item)) (response-code (pk 'response (http-get url)))))))))) -(test-equal "with cache, cache bypass" - 200 - (call-with-temporary-directory - (lambda (cache) - (let ((thread (with-separate-output-ports - (call-with-new-thread - (lambda () - (guix-publish "--port=6788" "-C" "gzip" - (string-append "--cache=" cache))))))) - (wait-until-ready 6788) - - (let* ((base "http://localhost:6788/") - (item (add-text-to-store %store "random" (random-text))) - (part (store-path-hash-part item)) - (narinfo (string-append base part ".narinfo")) - (nar (string-append base "nar/gzip/" (basename item))) - (cached (string-append cache "/gzip/" (basename item) - ".narinfo"))) - ;; We're below the default cache bypass threshold, so NAR and NARINFO - ;; should immediately return 200. The NARINFO request should trigger - ;; caching, and the next request to NAR should return 200 as well. - (and (let ((response (pk 'r1 (http-get nar)))) - (and (= 200 (response-code response)) - (not (response-content-length response)))) ;not known - (= 200 (response-code (http-get narinfo))) - (begin - (wait-for-file cached) - (let ((response (pk 'r2 (http-get nar)))) - (and (> (response-content-length response) - (stat:size (stat item))) - (response-code response)))))))))) - -(test-equal "with cache, cache bypass, unmapped hash part" - 200 - - ;; This test reproduces the bug described in : - ;; the daemon connection would be closed as a side effect of a nar request - ;; for a non-existing file name. - (call-with-temporary-directory - (lambda (cache) - (let ((thread (with-separate-output-ports - (call-with-new-thread - (lambda () - (guix-publish "--port=6787" "-C" "gzip" - (string-append "--cache=" cache))))))) - (wait-until-ready 6787) - - (let* ((base "http://localhost:6787/") - (item (add-text-to-store %store "random" (random-text))) - (part (store-path-hash-part item)) - (narinfo (string-append base part ".narinfo")) - (nar (string-append base "nar/gzip/" (basename item))) - (cached (string-append cache "/gzip/" (basename item) - ".narinfo"))) - ;; The first response used to be 500 and to terminate the daemon - ;; connection as a side effect. - (and (= (response-code - (http-get (string-append base "nar/gzip/" - (make-string 32 #\e) - "-does-not-exist"))) - 404) - (= 200 (response-code (http-get nar))) - (= 200 (response-code (http-get narinfo))) - (begin - (wait-for-file cached) - (response-code (http-get nar))))))))) - (test-equal "/log/NAME" `(200 #t application/x-bzip2) (let ((drv (run-with-store %store