From unknown Wed Jun 18 23:13:32 2025 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-Mailer: MIME-tools 5.509 (Entity 5.509) Content-Type: text/plain; charset=utf-8 From: bug#47283 <47283@debbugs.gnu.org> To: bug#47283 <47283@debbugs.gnu.org> Subject: Status: Performance regression in narinfo fetching Reply-To: bug#47283 <47283@debbugs.gnu.org> Date: Thu, 19 Jun 2025 06:13:32 +0000 retitle 47283 Performance regression in narinfo fetching reassign 47283 guix submitter 47283 Ludovic Court=C3=A8s severity 47283 important thanks From debbugs-submit-bounces@debbugs.gnu.org Sat Mar 20 13:38:56 2021 Received: (at submit) by debbugs.gnu.org; 20 Mar 2021 17:38:56 +0000 Received: from localhost ([127.0.0.1]:53099 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lNfYt-0005gQ-EJ for submit@debbugs.gnu.org; Sat, 20 Mar 2021 13:38:56 -0400 Received: from lists.gnu.org ([209.51.188.17]:58226) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lNfYp-0005gG-Lv for submit@debbugs.gnu.org; Sat, 20 Mar 2021 13:38:54 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:37248) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lNfYk-0006xn-ER for bug-guix@gnu.org; Sat, 20 Mar 2021 13:38:51 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:56957) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lNfYi-0003jH-6v for bug-guix@gnu.org; Sat, 20 Mar 2021 13:38:45 -0400 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=33620 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1lNfYf-0007Mn-Ru for bug-guix@gnu.org; Sat, 20 Mar 2021 13:38:42 -0400 From: =?utf-8?Q?Ludovic_Court=C3=A8s?= To: Subject: Performance regression in narinfo fetching X-Debbugs-Cc: Christopher Baines X-URL: http://www.fdn.fr/~lcourtes/ X-Revolutionary-Date: 30 =?utf-8?Q?Vent=C3=B4se?= an 229 de la =?utf-8?Q?R?= =?utf-8?Q?=C3=A9volution?= X-PGP-Key-ID: 0x090B11993D9AEBB5 X-PGP-Key: http://www.fdn.fr/~lcourtes/ludovic.asc X-PGP-Fingerprint: 3CE4 6455 8A84 FDC6 9DB4 0CFB 090B 1199 3D9A EBB5 X-OS: x86_64-pc-linux-gnu Date: Sat, 20 Mar 2021 18:38:39 +0100 Message-ID: <87ft0p67z4.fsf@inria.fr> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: submit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hello! As reported on guix-devel, =E2=80=98guix weather=E2=80=99 has become extrem= ely slow. Specifically, in the narinfo-fetching phase, it runs at 100% CPU, even though that part should be network-bound (pipelined HTTP GETs). A profile of the =E2=80=98report-server-coverage=E2=80=99 call would show t= his: --8<---------------cut here---------------start------------->8--- % cumulative self=20=20=20=20=20=20=20=20=20=20=20=20=20 time seconds seconds procedure 62.50 1.06 1.06 fluid-ref* 6.25 0.11 0.11 regexp-exec 3.13 0.05 0.05 ice-9/boot-9.scm:1738:4:throw 2.08 0.04 0.04 string-index 2.08 0.04 0.04 write 1.04 568.08 0.02 ice-9/boot-9.scm:1673:4:with-exception-handler 1.04 0.02 0.02 %read-line 1.04 0.02 0.02 guix/ci.scm:78:0:json->build 1.04 0.02 0.02 string-append --8<---------------cut here---------------end--------------->8--- More than half of the time spent in =E2=80=98fluid-ref*=E2=80=99=E2=80=94so= unds fishy. Where does that that call come from? There seems to be a single caller, in boot-9.scm: (define* (raise-exception exn #:key (continuable? #f)) (define (capture-current-exception-handlers) ;; FIXME: This is quadratic. (let lp ((depth 0)) (let ((h (fluid-ref* %exception-handler depth))) (if h (cons h (lp (1+ depth))) (list fallback-exception-handler))))) ;; =E2=80=A6 ) We must be abusing exceptions somewhere=E2=80=A6 Indeed, there=E2=80=99s one place on the hot path where we install exception handlers: in =E2=80=98http-multiple-get=E2=80=99 (from commit 205833b72c5517915a47a50dbe28e7024dc74e57). I don=E2=80=99t think it=E2=80= =99s needed, is it? (But if it is, let=E2=80=99s find another approach, this one is prohibitively expensive.) A simple performance test is: rm -rf ~/.cache/guix/substitute/ time ./pre-inst-env guix weather $(guix package -A|head -500| cut -f1) After removing this =E2=80=98catch=E2=80=99 in =E2=80=98http-multiple-get= =E2=80=99, the profile is flatter: --8<---------------cut here---------------start------------->8--- % cumulative self=20=20=20=20=20=20=20=20=20=20=20=20=20 time seconds seconds procedure 8.33 0.07 0.07 string-index 8.33 0.07 0.07 regexp-exec 5.56 0.05 0.05 anon #x154af88 5.56 0.05 0.05 write 5.56 0.05 0.05 string-tokenize 5.56 0.05 0.05 read-char 5.56 0.05 0.05 set-certificate-credentials-x509-trust-data! 5.56 0.05 0.05 %read-line --8<---------------cut here---------------end--------------->8--- There=E2=80=99s also this =E2=80=98call-with-connection-error-handling=E2= =80=99 call in (guix substitute), around an =E2=80=98http-multiple-get=E2=80=99 call, that may n= ot be justified. Attached is a diff of the tweaks I made to test this. WDYT, Chris? Ludo=E2=80=99. --=-=-= Content-Type: text/x-patch Content-Disposition: inline diff --git a/guix/http-client.scm b/guix/http-client.scm index 4b4c14ed0b..a28523201e 100644 --- a/guix/http-client.scm +++ b/guix/http-client.scm @@ -219,42 +219,21 @@ returning." (remainder (connect p remainder result)))) ((head tail ...) - (catch #t - (lambda () - (let* ((resp (read-response p)) - (body (response-body-port resp)) - (result (proc head resp body result))) - ;; The server can choose to stop responding at any time, - ;; in which case we have to try again. Check whether - ;; that is the case. Note that even upon "Connection: - ;; close", we can read from BODY. - (match (assq 'connection (response-headers resp)) - (('connection 'close) - (close-port p) - (connect #f ;try again - (drop requests (+ 1 processed)) - result)) - (_ - (loop tail (+ 1 processed) result))))) ;keep going - (lambda (key . args) - ;; If PORT was cached and the server closed the connection - ;; in the meantime, we get EPIPE. In that case, open a - ;; fresh connection and retry. We might also get - ;; 'bad-response or a similar exception from (web response) - ;; later on, once we've sent the request, or a - ;; ERROR/INVALID-SESSION from GnuTLS. - (if (or (and (eq? key 'system-error) - (= EPIPE (system-error-errno `(,key ,@args)))) - (and (eq? key 'gnutls-error) - (eq? (first args) error/invalid-session)) - (memq key - '(bad-response bad-header bad-header-component))) - (begin - (close-port p) - (connect #f ; try again - (drop requests (+ 1 processed)) - result)) - (apply throw key args)))))))))) + (let* ((resp (read-response p)) + (body (response-body-port resp)) + (result (proc head resp body result))) + ;; The server can choose to stop responding at any time, + ;; in which case we have to try again. Check whether + ;; that is the case. Note that even upon "Connection: + ;; close", we can read from BODY. + (match (assq 'connection (response-headers resp)) + (('connection 'close) + (close-port p) + (connect #f ;try again + (drop requests (+ 1 processed)) + result)) + (_ + (loop tail (+ 1 processed) result)))))))))) ;;; diff --git a/guix/scripts/weather.scm b/guix/scripts/weather.scm index 5164fe0494..3d8d50d5e3 100644 --- a/guix/scripts/weather.scm +++ b/guix/scripts/weather.scm @@ -184,9 +184,10 @@ Return the coverage ratio, an exact number between 0 and 1." (let/time ((time narinfos requests-made (lookup-narinfos server items - #:make-progress-reporter - (lambda* (total #:key url #:allow-other-keys) - (progress-reporter/bar total))))) + ;; #:make-progress-reporter + ;; (lambda* (total #:key url #:allow-other-keys) + ;; (progress-reporter/bar total)) + ))) (format #t "~a~%" server) (let ((obtained (length narinfos)) (requested (length items)) @@ -504,6 +505,7 @@ SERVER. Display information for packages with at least THRESHOLD dependents." ;;; Entry point. ;;; +(use-modules (statprof)) (define-command (guix-weather . args) (synopsis "report on the availability of pre-built package binaries") @@ -551,9 +553,11 @@ SERVER. Display information for packages with at least THRESHOLD dependents." (exit (every* (lambda (server) (define coverage - (report-server-coverage server items - #:display-missing? - (assoc-ref opts 'display-missing?))) + (statprof + (lambda () + (report-server-coverage server items + #:display-missing? + (assoc-ref opts 'display-missing?))))) (match (assoc-ref opts 'coverage) (#f #f) (threshold diff --git a/guix/substitutes.scm b/guix/substitutes.scm index 08f8c24efd..04bf70caaa 100644 --- a/guix/substitutes.scm +++ b/guix/substitutes.scm @@ -59,8 +59,6 @@ #:use-module (guix http-client) #:export (%narinfo-cache-directory - call-with-connection-error-handling - lookup-narinfos lookup-narinfos/diverse)) @@ -235,14 +233,11 @@ if file doesn't exist, and the narinfo otherwise." (let* ((requests (map (cut narinfo-request url <>) paths)) (result (begin (start-progress-reporter! progress-reporter) - (call-with-connection-error-handling - uri - (lambda () - (http-multiple-get uri - handle-narinfo-response '() - requests - #:open-connection open-connection - #:verify-certificate? #f)))))) + (http-multiple-get uri + handle-narinfo-response '() + requests + #:open-connection open-connection + #:verify-certificate? #f)))) (stop-progress-reporter! progress-reporter) result)) ((file #f) --=-=-=-- From debbugs-submit-bounces@debbugs.gnu.org Sat Mar 20 16:32:42 2021 Received: (at 47283) by debbugs.gnu.org; 20 Mar 2021 20:32:42 +0000 Received: from localhost ([127.0.0.1]:53180 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lNiH3-0001Kg-KP for submit@debbugs.gnu.org; Sat, 20 Mar 2021 16:32:41 -0400 Received: from mira.cbaines.net ([212.71.252.8]:59174) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lNiH0-0001KX-Qd for 47283@debbugs.gnu.org; Sat, 20 Mar 2021 16:32:39 -0400 Received: from localhost (unknown [IPv6:2a02:8010:68c1:0:8ac0:b4c7:f5c8:7caa]) by mira.cbaines.net (Postfix) with ESMTPSA id DCF3727BC57; Sat, 20 Mar 2021 20:32:37 +0000 (GMT) Received: from capella (localhost [127.0.0.1]) by localhost (OpenSMTPD) with ESMTP id 411e1e2a; Sat, 20 Mar 2021 20:32:37 +0000 (UTC) References: <87ft0p67z4.fsf@inria.fr> User-agent: mu4e 1.4.15; emacs 27.1 From: Christopher Baines To: Ludovic =?utf-8?Q?Court=C3=A8s?= Subject: Re: bug#47283: Performance regression in narinfo fetching In-reply-to: <87ft0p67z4.fsf@inria.fr> Date: Sat, 20 Mar 2021 20:32:35 +0000 Message-ID: <874kh5d0rg.fsf@cbaines.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" X-Spam-Score: -0.0 (/) X-Debbugs-Envelope-To: 47283 Cc: 47283@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Ludovic Court=C3=A8s writes: > As reported on guix-devel, =E2=80=98guix weather=E2=80=99 has become extr= emely slow. > Specifically, in the narinfo-fetching phase, it runs at 100% CPU, even > though that part should be network-bound (pipelined HTTP GETs). > > A profile of the =E2=80=98report-server-coverage=E2=80=99 call would show= this: > > --8<---------------cut here---------------start------------->8--- > % cumulative self=20=20=20=20=20=20=20=20=20=20=20=20=20 > time seconds seconds procedure > 62.50 1.06 1.06 fluid-ref* > 6.25 0.11 0.11 regexp-exec > 3.13 0.05 0.05 ice-9/boot-9.scm:1738:4:throw > 2.08 0.04 0.04 string-index > 2.08 0.04 0.04 write > 1.04 568.08 0.02 ice-9/boot-9.scm:1673:4:with-exception-handler > 1.04 0.02 0.02 %read-line > 1.04 0.02 0.02 guix/ci.scm:78:0:json->build > 1.04 0.02 0.02 string-append > --8<---------------cut here---------------end--------------->8--- > > More than half of the time spent in =E2=80=98fluid-ref*=E2=80=99=E2=80=94= sounds fishy. > > Where does that that call come from? There seems to be a single caller, > in boot-9.scm: > > (define* (raise-exception exn #:key (continuable? #f)) > (define (capture-current-exception-handlers) > ;; FIXME: This is quadratic. > (let lp ((depth 0)) > (let ((h (fluid-ref* %exception-handler depth))) > (if h > (cons h (lp (1+ depth))) > (list fallback-exception-handler))))) > ;; =E2=80=A6 > ) > > We must be abusing exceptions somewhere=E2=80=A6 > > Indeed, there=E2=80=99s one place on the hot path where we install except= ion > handlers: in =E2=80=98http-multiple-get=E2=80=99 (from commit > 205833b72c5517915a47a50dbe28e7024dc74e57). I don=E2=80=99t think it=E2= =80=99s needed, > is it? (But if it is, let=E2=80=99s find another approach, this one is > prohibitively expensive.) I think the exception handling has moved around, but I guess the exceptions that could be caught in http-multiple-get could happen, right? I am really just guessing here, as Guile doesn't help tell you about possible exceptions, and I haven't spent enough time to read all the possible code involved to find out if these are definitely possible. > A simple performance test is: > > rm -rf ~/.cache/guix/substitute/ > time ./pre-inst-env guix weather $(guix package -A|head -500| cut -f1) > > After removing this =E2=80=98catch=E2=80=99 in =E2=80=98http-multiple-get= =E2=80=99, the profile is > flatter: > > --8<---------------cut here---------------start------------->8--- > % cumulative self > time seconds seconds procedure > 8.33 0.07 0.07 string-index > 8.33 0.07 0.07 regexp-exec > 5.56 0.05 0.05 anon #x154af88 > 5.56 0.05 0.05 write > 5.56 0.05 0.05 string-tokenize > 5.56 0.05 0.05 read-char > 5.56 0.05 0.05 set-certificate-credentials-x509-trust-data! > 5.56 0.05 0.05 %read-line > --8<---------------cut here---------------end--------------->8--- > > There=E2=80=99s also this =E2=80=98call-with-connection-error-handling=E2= =80=99 call in (guix > substitute), around an =E2=80=98http-multiple-get=E2=80=99 call, that may= not be > justified. > > Attached is a diff of the tweaks I made to test this. > > WDYT, Chris? I haven't looked in to this yet, but maybe it would be possible to adjust the code so that it doesn't perform so badly, but still tries to handle possible exceptions. The two ideas I have is to rewrite the (let ...) bit in terms of a fold, maybe that would perform better, or stop using let for iteration and setup the exception handling, then process each request, using set! to update the state. I haven't tested either of these. It's good to know that Guile exception handling can be excessively expensive though, I wouldn't have expected it to beat out anything over the network in terms of the performance penalty. --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQKlBAEBCgCPFiEEPonu50WOcg2XVOCyXiijOwuE9XcFAmBWW+NfFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcRHG1haWxAY2Jh aW5lcy5uZXQACgkQXiijOwuE9Xenow//RkiKPMtSiWGvnn/tEVC1d+o/4tK/SrKy L5QbyF9k/oOCsCGWyZKlgFhrjctcoYs3He4KY9T8ba2xy3YHN3m5Xm9aZcTS7fgH 8AwjiMZ/Edw+qF55+wXYcxGF8ch8SfMib3rRnBH5K0yhQ1KH2wZzDTCp8MdBfMzl 5PwMbhLpKTlNW82ZuGqDGj50/Ca5QMg4XRr7r1ACQR4W8ee0VilF1K/Kp7QTT0B7 ikwOLvxaqsVwlSo7Kyvu3FIiSeIlWXFRjSTtAUSEtBzREoyp+oh+7YGWJ4buaz5x WAl0/cfs6Blf5Y9MrEaCkHV0kvHzknelWYoR1IBMBAOhwksU1mOKD3I45fhixTYg E4oNkFN9ug9WPzz522skbWyoHuQH6zRLh1aoXpC2xSydcu7hXSyA+NZ5bpBWIzRI rMsHhryV/kA7ZhOQKUSbJIdpu3mFYYmuCAeAdK2oO5FF7oRZ3yb4samHebWFMog+ 7U71Y+iR0ga12NmUzN1sBxfTFHa2ynlRlyuhF/H4KiFa09Z9xzsHRu02dPmh5u0q UXz2twCd/pVg0OmnPpFvedw9NaBoxNViawfHRxLz5C8xqpbpXqM1j0OJcvSqfXC3 OJDhsRNLZrnusLFi31ydxkTr+R+f03PPgmJYAG9ay8iIpFKOt9HLDLQeCPReexdF vBxzyBRUhtM= =gCEB -----END PGP SIGNATURE----- --=-=-=-- From debbugs-submit-bounces@debbugs.gnu.org Sat Mar 20 16:54:53 2021 Received: (at control) by debbugs.gnu.org; 20 Mar 2021 20:54:53 +0000 Received: from localhost ([127.0.0.1]:53189 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lNicW-0001qR-W8 for submit@debbugs.gnu.org; Sat, 20 Mar 2021 16:54:53 -0400 Received: from eggs.gnu.org ([209.51.188.92]:38900) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lNicV-0001qC-Tc for control@debbugs.gnu.org; Sat, 20 Mar 2021 16:54:52 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:59221) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lNicQ-0003kz-Lu for control@debbugs.gnu.org; Sat, 20 Mar 2021 16:54:46 -0400 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=34688 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1lNicQ-0000rp-4l for control@debbugs.gnu.org; Sat, 20 Mar 2021 16:54:46 -0400 Date: Sat, 20 Mar 2021 21:54:45 +0100 Message-Id: <87y2eh4kbu.fsf@gnu.org> To: control@debbugs.gnu.org From: =?utf-8?Q?Ludovic_Court=C3=A8s?= Subject: control message for bug #47283 MIME-version: 1.0 Content-type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: control X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) severity 47283 important quit From debbugs-submit-bounces@debbugs.gnu.org Sat Mar 20 20:48:09 2021 Received: (at 47283) by debbugs.gnu.org; 21 Mar 2021 00:48:09 +0000 Received: from localhost ([127.0.0.1]:53360 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lNmGH-0003NJ-7Q for submit@debbugs.gnu.org; Sat, 20 Mar 2021 20:48:09 -0400 Received: from mira.cbaines.net ([212.71.252.8]:59670) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lNmGE-0003N9-FA for 47283@debbugs.gnu.org; Sat, 20 Mar 2021 20:48:07 -0400 Received: from localhost (unknown [IPv6:2a02:8010:68c1:0:8ac0:b4c7:f5c8:7caa]) by mira.cbaines.net (Postfix) with ESMTPSA id 5673627BC57; Sun, 21 Mar 2021 00:48:04 +0000 (GMT) Received: from capella (localhost [127.0.0.1]) by localhost (OpenSMTPD) with ESMTP id bfb097a1; Sun, 21 Mar 2021 00:48:04 +0000 (UTC) References: <87ft0p67z4.fsf@inria.fr> <874kh5d0rg.fsf@cbaines.net> User-agent: mu4e 1.4.15; emacs 27.1 From: Christopher Baines To: 47283@debbugs.gnu.org Subject: Re: bug#47283: Performance regression in narinfo fetching In-reply-to: <874kh5d0rg.fsf@cbaines.net> Date: Sun, 21 Mar 2021 00:48:03 +0000 Message-ID: <871rc9coxo.fsf@cbaines.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" X-Spam-Score: -0.0 (/) X-Debbugs-Envelope-To: 47283 Cc: Ludovic =?utf-8?Q?Court=C3=A8s?= X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) --=-=-= Content-Type: text/plain Christopher Baines writes: > I haven't looked in to this yet, but maybe it would be possible to > adjust the code so that it doesn't perform so badly, but still tries to > handle possible exceptions. > > The two ideas I have is to rewrite the (let ...) bit in terms of a fold, > maybe that would perform better, or stop using let for iteration and > setup the exception handling, then process each request, using set! to > update the state. I haven't tested either of these. I tried something, neither of these things, but just not calling (loop ...) within the catch block. I don't know why this might work, but it seems to make guix weather much faster. Here's the patch [1], I've just realised it's broken, as it'll loose the result value (and use an old one) when the connection is closed. I'll send a updated patch without this issue in a moment. 1: https://issues.guix.gnu.org/47288 --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQKlBAEBCgCPFiEEPonu50WOcg2XVOCyXiijOwuE9XcFAmBWl8NfFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcRHG1haWxAY2Jh aW5lcy5uZXQACgkQXiijOwuE9XfIpw//QGNGAFecZaN9H8GjP/XTfT7dU2MB2kcB qS6sIMtgPkBNRLLktK2HkQL9KQq2f8W0hTw+Kh7blzHPnBQwauwUDLukscSXxTH+ AOJMJ7WTOfgQoy2kAT3HLtzG7/Ww76arA6v1TzizttTSXz9dxbbFV9CnNRAtQqcW r4St0bnT1rfAcW1SHDrbl1Bs2ORxWlxhAjjwRNM6psK0QG/BWzft00JyfbiQT8YF qwG7zaJwiyOzhCJ2UWnrDlfapax0u4mzObbf+mSctIYynSFk3LJY/wEU35aWkkoZ coZPBaf2udXN4qzwRb26Yj8JduC6liKjbvzzMQiU7oSZ9pSNqADhFUJYrjblRURV GVA2h2pSk1fVOk/OikqEuyfPx5Kj0fsbQ1IMtmaGcbO1vNUhxISYRspwuFC1hCYv Cqw2kJ+0MbqggMANrw2qqszhMYb4zIqYkJDOeLh4rjUNluyzxIV6TWK76B7O7dz1 t6HnI66AigNosEMyH3u8ViATIe3Od1jmz7w7Q62t5QqiPPkKnF4CVd87p4YUrv2z oQ5piDwgNouAeBklsHNd6mXaetYLXtHg6GHEjBDZUeNRKciJ63+DAzAWd5ojLfVu cxUqR6Ag7p9zRVlUSIC6BTpKmJD7gC6RtmjeAQeAtDbDUSfo1+2+iq+1QEhcoA65 Xd/zFiFlE0U= =hdwE -----END PGP SIGNATURE----- --=-=-=-- From debbugs-submit-bounces@debbugs.gnu.org Sun Mar 21 17:10:52 2021 Received: (at 47283) by debbugs.gnu.org; 21 Mar 2021 21:10:53 +0000 Received: from localhost ([127.0.0.1]:55556 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lO5LY-0001k5-MV for submit@debbugs.gnu.org; Sun, 21 Mar 2021 17:10:52 -0400 Received: from eggs.gnu.org ([209.51.188.92]:50186) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lO5LU-0001jn-VH for 47283@debbugs.gnu.org; Sun, 21 Mar 2021 17:10:51 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:44440) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lO5LP-0005Yi-9E; Sun, 21 Mar 2021 17:10:43 -0400 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=39076 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1lO5LO-0002pQ-NB; Sun, 21 Mar 2021 17:10:42 -0400 From: =?utf-8?Q?Ludovic_Court=C3=A8s?= To: Christopher Baines Subject: Re: bug#47283: Performance regression in narinfo fetching References: <87ft0p67z4.fsf@inria.fr> <874kh5d0rg.fsf@cbaines.net> <871rc9coxo.fsf@cbaines.net> X-URL: http://www.fdn.fr/~lcourtes/ X-Revolutionary-Date: 1 Germinal an 229 de la =?utf-8?Q?R=C3=A9volution?= X-PGP-Key-ID: 0x090B11993D9AEBB5 X-PGP-Key: http://www.fdn.fr/~lcourtes/ludovic.asc X-PGP-Fingerprint: 3CE4 6455 8A84 FDC6 9DB4 0CFB 090B 1199 3D9A EBB5 X-OS: x86_64-pc-linux-gnu Date: Sun, 21 Mar 2021 22:10:40 +0100 In-Reply-To: <871rc9coxo.fsf@cbaines.net> (Christopher Baines's message of "Sun, 21 Mar 2021 00:48:03 +0000") Message-ID: <87o8fc1acv.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 47283 Cc: 47283@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) Hi! Christopher Baines skribis: > Christopher Baines writes: > >> I haven't looked in to this yet, but maybe it would be possible to >> adjust the code so that it doesn't perform so badly, but still tries to >> handle possible exceptions. >> >> The two ideas I have is to rewrite the (let ...) bit in terms of a fold, >> maybe that would perform better, or stop using let for iteration and >> setup the exception handling, then process each request, using set! to >> update the state. I haven't tested either of these. > > I tried something, neither of these things, but just not calling (loop > ...) within the catch block. I don't know why this might work, but it > seems to make guix weather much faster. Oh yes, that=E2=80=99s also because calling =E2=80=98loop=E2=80=99 from wit= hin =E2=80=98catch=E2=80=99 made it a non-tail call, so we kept accumulating exception handlers, and the =E2=80= =98lp=E2=80=99 loop in =E2=80=98raise-exception=E2=80=99 would have an ever increasing lis= t of handlers to traverse. > Here's the patch [1], I've just realised it's broken, as it'll loose the > result value (and use an old one) when the connection is closed. I'll > send a updated patch without this issue in a moment. > > 1: https://issues.guix.gnu.org/47288 OK, thanks. I=E2=80=99ll reply to your other message first. :-) Ludo=E2=80=99. From debbugs-submit-bounces@debbugs.gnu.org Sun Mar 21 17:22:21 2021 Received: (at 47283) by debbugs.gnu.org; 21 Mar 2021 21:22:21 +0000 Received: from localhost ([127.0.0.1]:55572 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lO5Wf-000224-Bz for submit@debbugs.gnu.org; Sun, 21 Mar 2021 17:22:21 -0400 Received: from eggs.gnu.org ([209.51.188.92]:51918) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lO5Wd-00021r-1s for 47283@debbugs.gnu.org; Sun, 21 Mar 2021 17:22:20 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:44535) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lO5WX-0002Nq-JX; Sun, 21 Mar 2021 17:22:13 -0400 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=39086 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1lO5WW-0003kt-SH; Sun, 21 Mar 2021 17:22:13 -0400 From: =?utf-8?Q?Ludovic_Court=C3=A8s?= To: Christopher Baines Subject: Re: bug#47283: Performance regression in narinfo fetching References: <87ft0p67z4.fsf@inria.fr> <874kh5d0rg.fsf@cbaines.net> X-URL: http://www.fdn.fr/~lcourtes/ X-Revolutionary-Date: 1 Germinal an 229 de la =?utf-8?Q?R=C3=A9volution?= X-PGP-Key-ID: 0x090B11993D9AEBB5 X-PGP-Key: http://www.fdn.fr/~lcourtes/ludovic.asc X-PGP-Fingerprint: 3CE4 6455 8A84 FDC6 9DB4 0CFB 090B 1199 3D9A EBB5 X-OS: x86_64-pc-linux-gnu Date: Sun, 21 Mar 2021 22:22:10 +0100 In-Reply-To: <874kh5d0rg.fsf@cbaines.net> (Christopher Baines's message of "Sat, 20 Mar 2021 20:32:35 +0000") Message-ID: <87czvs19tp.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 47283 Cc: 47283@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Christopher Baines skribis: > Ludovic Court=C3=A8s writes: > >> Indeed, there=E2=80=99s one place on the hot path where we install excep= tion >> handlers: in =E2=80=98http-multiple-get=E2=80=99 (from commit >> 205833b72c5517915a47a50dbe28e7024dc74e57). I don=E2=80=99t think it=E2= =80=99s needed, >> is it? (But if it is, let=E2=80=99s find another approach, this one is >> prohibitively expensive.) > > I think the exception handling has moved around, but I guess the > exceptions that could be caught in http-multiple-get could happen, > right? I am really just guessing here, as Guile doesn't help tell you > about possible exceptions, and I haven't spent enough time to read all > the possible code involved to find out if these are definitely possible. Yeah. Commit 205833b72c5517915a47a50dbe28e7024dc74e57 added a =E2=80=98catch=E2= =80=99 block that catches the same things as =E2=80=98with-cached-connection=E2=80=99 di= d (it would be better to not duplicate it IMO). That includes EPIPE, gnutls-error, bad-response & co. Earlier, commit be5a75ebb5988b87b2392e2113f6590f353dd6cd (=E2=80=9Csubstitu= te: Reuse connections for '--query'.=E2=80=9D) did not add such a =E2=80=98catc= h=E2=80=99 block in =E2=80=98http-multiple-get=E2=80=99. Instead, it wrapped its call in =E2= =80=98do-fetch=E2=80=99 in =E2=80=98fetch-narinfos=E2=80=99: --=-=-= Content-Type: text/x-patch Content-Disposition: inline (define (do-fetch uri) (case (and=> uri uri-scheme) ((http https) - (let ((requests (map (cut narinfo-request url <>) paths))) - (match (open-connection-for-uri/maybe uri) - (#f - '()) - (port - (update-progress!) ;; Note: Do not check HTTPS server certificates to avoid depending ;; on the X.509 PKI. We can do it because we authenticate ;; narinfos, which provides a much stronger guarantee. - (let ((result (http-multiple-get uri + (let* ((requests (map (cut narinfo-request url <>) paths)) + (result (call-with-cached-connection uri + (lambda (port) + (if port + (begin + (update-progress!) + (http-multiple-get uri handle-narinfo-response '() requests + #:open-connection + open-connection-for-uri/cached #:verify-certificate? #f - #:port port))) --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable This bit is still there in current =E2=80=98master=E2=80=99, so I think it= =E2=80=99s not necessary to catch these exceptions in =E2=80=98http-multiple-get=E2=80=99 = itself, and I would just remove the =E2=80=98catch=E2=80=99 wrap altogether. WDYT? Thanks, Ludo=E2=80=99. --=-=-=-- From debbugs-submit-bounces@debbugs.gnu.org Tue Mar 23 16:47:18 2021 Received: (at 47283) by debbugs.gnu.org; 23 Mar 2021 20:47:18 +0000 Received: from localhost ([127.0.0.1]:33193 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lOnvp-0001EW-Sf for submit@debbugs.gnu.org; Tue, 23 Mar 2021 16:47:18 -0400 Received: from mira.cbaines.net ([212.71.252.8]:57424) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lOnvo-0001EP-Ij for 47283@debbugs.gnu.org; Tue, 23 Mar 2021 16:47:17 -0400 Received: from localhost (unknown [IPv6:2a02:8010:68c1:0:8ac0:b4c7:f5c8:7caa]) by mira.cbaines.net (Postfix) with ESMTPSA id 6382627BC59; Tue, 23 Mar 2021 20:47:15 +0000 (GMT) Received: from capella (localhost [127.0.0.1]) by localhost (OpenSMTPD) with ESMTP id e8a34252; Tue, 23 Mar 2021 20:47:15 +0000 (UTC) References: <87ft0p67z4.fsf@inria.fr> <874kh5d0rg.fsf@cbaines.net> <87czvs19tp.fsf@gnu.org> User-agent: mu4e 1.4.15; emacs 27.1 From: Christopher Baines To: Ludovic =?utf-8?Q?Court=C3=A8s?= Subject: Re: bug#47283: Performance regression in narinfo fetching In-reply-to: <87czvs19tp.fsf@gnu.org> Date: Tue, 23 Mar 2021 20:47:12 +0000 Message-ID: <87k0pxbnsf.fsf@cbaines.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" X-Spam-Score: -0.0 (/) X-Debbugs-Envelope-To: 47283 Cc: 47283@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Ludovic Court=C3=A8s writes: > Christopher Baines skribis: > >> Ludovic Court=C3=A8s writes: >> >>> Indeed, there=E2=80=99s one place on the hot path where we install exce= ption >>> handlers: in =E2=80=98http-multiple-get=E2=80=99 (from commit >>> 205833b72c5517915a47a50dbe28e7024dc74e57). I don=E2=80=99t think it=E2= =80=99s needed, >>> is it? (But if it is, let=E2=80=99s find another approach, this one is >>> prohibitively expensive.) >> >> I think the exception handling has moved around, but I guess the >> exceptions that could be caught in http-multiple-get could happen, >> right? I am really just guessing here, as Guile doesn't help tell you >> about possible exceptions, and I haven't spent enough time to read all >> the possible code involved to find out if these are definitely possible. > > Yeah. > > Commit 205833b72c5517915a47a50dbe28e7024dc74e57 added a =E2=80=98catch=E2= =80=99 block > that catches the same things as =E2=80=98with-cached-connection=E2=80=99 = did (it would > be better to not duplicate it IMO). That includes EPIPE, gnutls-error, > bad-response & co. So, my intention here was to move the error handling, to allow separating out the connection caching code from the code I wanted to move out to the (guix substitutes) module. I don't think there's currently duplication in the error handling for the code path involving http-multiple-get currently, at least for the exceptions in question here. > Earlier, commit be5a75ebb5988b87b2392e2113f6590f353dd6cd (=E2=80=9Csubsti= tute: > Reuse connections for '--query'.=E2=80=9D) did not add such a =E2=80=98ca= tch=E2=80=99 block in > =E2=80=98http-multiple-get=E2=80=99. Instead, it wrapped its call in =E2= =80=98do-fetch=E2=80=99 in > =E2=80=98fetch-narinfos=E2=80=99: > > (define (do-fetch uri) > (case (and=3D> uri uri-scheme) > ((http https) > - (let ((requests (map (cut narinfo-request url <>) paths))) > - (match (open-connection-for-uri/maybe uri) > - (#f > - '()) > - (port > - (update-progress!) > ;; Note: Do not check HTTPS server certificates to avoid depending > ;; on the X.509 PKI. We can do it because we authenticate > ;; narinfos, which provides a much stronger guarantee. > - (let ((result (http-multiple-get uri > + (let* ((requests (map (cut narinfo-request url <>) paths)) > + (result (call-with-cached-connection uri > + (lambda (port) > + (if port > + (begin > + (update-progress!) > + (http-multiple-get uri > handle-narinfo-resp= onse '() > requests > + #:open-connection > + open-connection-for= -uri/cached > #:verify-certificat= e? #f > - #:port port))) > > This bit is still there in current =E2=80=98master=E2=80=99, so I think i= t=E2=80=99s not > necessary to catch these exceptions in =E2=80=98http-multiple-get=E2=80= =99 itself, and I > would just remove the =E2=80=98catch=E2=80=99 wrap altogether. > > WDYT? I'm not sure what you're referring to as still being there on the master branch? Looking at the changes to this particular code path resulting from the changes I've made recently, starting at lookup-narinfos, before: - lookup-narinfos calls fetch-narinfos, which calls do-fetch - call-with-cached-connection is used, which catches a number of exceptions relating to requests, and will retry PROC once upon a matching exception - open-connection-for-uri/maybe is also used, which is like open-connection-for-uri/cached, except it includes error handling for establishing connections to substitute servers - http-multiple-get doesn't include error handling After: - lookup-narinfos calls fetch-narinfos, which calls do-fetch - call-with-connection-error-handling is used, which performs the same role as the error handling previously within open-connection-for-uri/maybe, catching exceptions relating to establishing connections to substitute servers - http-multiple-get now includes error handling similar to what was previously done by call-with-cached-connection, although it's more complicated since it's done with knowledge of what http-multiple-get is doing I think that the error handling now in http-multiple-get isn't covered elsewhere. Moving this error handling back in to fetch-narinfos is possible, but then we'd be back to handling connection caching in that code, and avoiding that led to this refactoring in the first place. Also, apart from the implementation problems, I do think that the error handling here is better than before. Previously, if you called lookup-narinfos, and a connection problem occurred, processing all the requests would start from scratch (as call-with-cached-connection calls PROC a second time), if a second connection error was to happen, well, call-with-cached-connection only handles one error, so that won't be caught. I think it's possible that http-multiple-get will be making thousands of requests, running guix weather with no cached results for example. The error handling in http-multiple-get is smarter than the previous approach, doing as little as possible again. It's also not limited to catching one exception. --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQKlBAEBCgCPFiEEPonu50WOcg2XVOCyXiijOwuE9XcFAmBaU9BfFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcRHG1haWxAY2Jh aW5lcy5uZXQACgkQXiijOwuE9XdvCxAAtsUzNiHvJL2S0jf1/VJUdqZw9VSb91BM GnyMd5hXQIe0ONd3r6XkdLdPpe9JT2Lh18U0F6fvJfk+zhDFWIAPZE9PiyweBarf 0ggJH45zr8nxbIrPp/dOuDvuCEhnEyovK0bPdvKQfzkC67ZQdlfkjnPjDKbtactA l2hgWdtoT7wN0QdwFpa45fQhbP1zHWh5Wd0kItyxSjM3B9xpnoYHP/IryuTUhtVd jx18SFdPKTHUYwWpESrjRM79tE53IfcChYz7b5toUgDnGSM63nZQHbLbmVcQXLrO m8rRhTcf3ZbDqg5qQr7RAQfwucO/1C9NIkP6Gjn0Hnk2e/q6fHdhLpjIDwmQy2S2 yIx28JcpKB4ua+ToyVDsEZGuJiacJan5P306B7LjrDVx8BRWF36Xt3PZEV2kHKrd uFwOlGe829zYsXtp45FnVRd4wHzP9uqDnQoh4f4vyLaoKqB/Wx4UNb0fqIJ4mY5Y lHIlbXIUpkEAb4ON9jm2D81bWUCFcfLTJj5mkrXer3J8II2pcg/Md7MgmyhXcbOa zYq5N3TyHaQs5PeWhuS3oXUePjnpxse0oAtH3V0cm8iGa/iggWUmC6+eu7b/X1v/ Fo3RXgu5/Zbx6xvVtrzSiq4s9CZKLh1qhUjSZBlG5SquHg2WJNSKZ3/Grwi+x78s ITZ9f0QQh4E= =O8Tt -----END PGP SIGNATURE----- --=-=-=-- From debbugs-submit-bounces@debbugs.gnu.org Wed Mar 24 10:51:58 2021 Received: (at 47283) by debbugs.gnu.org; 24 Mar 2021 14:51:58 +0000 Received: from localhost ([127.0.0.1]:35782 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lP4rV-0001Ws-VV for submit@debbugs.gnu.org; Wed, 24 Mar 2021 10:51:58 -0400 Received: from eggs.gnu.org ([209.51.188.92]:35898) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lP4rT-0001We-Ks for 47283@debbugs.gnu.org; Wed, 24 Mar 2021 10:51:56 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:57146) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lP4rN-00021C-U6; Wed, 24 Mar 2021 10:51:49 -0400 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=57324 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1lP4rN-0001yD-AU; Wed, 24 Mar 2021 10:51:49 -0400 From: =?utf-8?Q?Ludovic_Court=C3=A8s?= To: Christopher Baines Subject: Re: bug#47283: Performance regression in narinfo fetching References: <87ft0p67z4.fsf@inria.fr> <874kh5d0rg.fsf@cbaines.net> <87czvs19tp.fsf@gnu.org> <87k0pxbnsf.fsf@cbaines.net> Date: Wed, 24 Mar 2021 15:51:47 +0100 In-Reply-To: <87k0pxbnsf.fsf@cbaines.net> (Christopher Baines's message of "Tue, 23 Mar 2021 20:47:12 +0000") Message-ID: <87y2eceha4.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 47283 Cc: 47283@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) Hi Chris, Christopher Baines skribis: > Ludovic Court=C3=A8s writes: [...] >> Earlier, commit be5a75ebb5988b87b2392e2113f6590f353dd6cd (=E2=80=9Csubst= itute: >> Reuse connections for '--query'.=E2=80=9D) did not add such a =E2=80=98c= atch=E2=80=99 block in >> =E2=80=98http-multiple-get=E2=80=99. Instead, it wrapped its call in = =E2=80=98do-fetch=E2=80=99 in >> =E2=80=98fetch-narinfos=E2=80=99: >> >> (define (do-fetch uri) >> (case (and=3D> uri uri-scheme) >> ((http https) >> - (let ((requests (map (cut narinfo-request url <>) paths))) >> - (match (open-connection-for-uri/maybe uri) >> - (#f >> - '()) >> - (port >> - (update-progress!) >> ;; Note: Do not check HTTPS server certificates to avoid dependi= ng >> ;; on the X.509 PKI. We can do it because we authenticate >> ;; narinfos, which provides a much stronger guarantee. >> - (let ((result (http-multiple-get uri >> + (let* ((requests (map (cut narinfo-request url <>) paths)) >> + (result (call-with-cached-connection uri >> + (lambda (port) >> + (if port >> + (begin >> + (update-progress!) >> + (http-multiple-get uri >> handle-narinfo-res= ponse '() >> requests >> + #:open-connection >> + open-connection-fo= r-uri/cached >> #:verify-certifica= te? #f >> - #:port port))) >> >> This bit is still there in current =E2=80=98master=E2=80=99, so I think = it=E2=80=99s not >> necessary to catch these exceptions in =E2=80=98http-multiple-get=E2=80= =99 itself, and I >> would just remove the =E2=80=98catch=E2=80=99 wrap altogether. >> >> WDYT? > > I'm not sure what you're referring to as still being there on the master > branch? On =E2=80=98master=E2=80=99, =E2=80=98do-fetch=E2=80=99 has its =E2=80=98ht= tp-multiple-get=E2=80=99 call wrapped in =E2=80=98call-with-connection-error-handling=E2=80=99, which is equivalent = to the change made in be5a75ebb5988b87b2392e2113f6590f353dd6cd and shown above. > Looking at the changes to this particular code path resulting from the > changes I've made recently, starting at lookup-narinfos, before: > > - lookup-narinfos calls fetch-narinfos, which calls do-fetch > > - call-with-cached-connection is used, which catches a number of > exceptions relating to requests, and will retry PROC once upon a > matching exception > > - open-connection-for-uri/maybe is also used, which is like > open-connection-for-uri/cached, except it includes error handling for > establishing connections to substitute servers > > - http-multiple-get doesn't include error handling > > After: > > - lookup-narinfos calls fetch-narinfos, which calls do-fetch > > - call-with-connection-error-handling is used, which performs the same > role as the error handling previously within > open-connection-for-uri/maybe, catching exceptions relating to > establishing connections to substitute servers > > - http-multiple-get now includes error handling similar to what was > previously done by call-with-cached-connection, although it's more > complicated since it's done with knowledge of what http-multiple-get > is doing > > I think that the error handling now in http-multiple-get isn't covered > elsewhere. Moving this error handling back in to fetch-narinfos is > possible, but then we'd be back to handling connection caching in that > code, and avoiding that led to this refactoring in the first place. The =E2=80=98http-multiple-get=E2=80=99 call in =E2=80=98fetch-narinfos=E2= =80=99 is already wrapped in =E2=80=98call-with-connection-error-handling=E2=80=99, so it seems we=E2=80= =99re good? > Also, apart from the implementation problems, I do think that the error > handling here is better than before. Previously, if you called > lookup-narinfos, and a connection problem occurred, processing all the > requests would start from scratch (as call-with-cached-connection calls > PROC a second time), if a second connection error was to happen, well, > call-with-cached-connection only handles one error, so that won't be > caught. Hmm true. However, can that second exception happen? Normally, if we get a first exception, we open a new connection and that one should not get another exception, unless something is wrong=E2=80=94in which case it= =E2=80=99s probably best to report it than to endlessly retry. WDYT? Even in tail call position, =E2=80=98catch=E2=80=99 calls introduce allocat= ions and extra work, so if we can avoid using one =E2=80=98catch=E2=80=99 per iterat= ion, that=E2=80=99s better. Thank you, Ludo=E2=80=99. From debbugs-submit-bounces@debbugs.gnu.org Sat Mar 27 17:58:50 2021 Received: (at 47283-done) by debbugs.gnu.org; 27 Mar 2021 21:58:50 +0000 Received: from localhost ([127.0.0.1]:45099 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lQGxG-0002V1-4f for submit@debbugs.gnu.org; Sat, 27 Mar 2021 17:58:50 -0400 Received: from eggs.gnu.org ([209.51.188.92]:56178) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lQGxD-0002Uk-DP; Sat, 27 Mar 2021 17:58:48 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:43631) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lQGx7-0007fi-Tc; Sat, 27 Mar 2021 17:58:41 -0400 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=43670 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1lQGx6-0005Mj-UD; Sat, 27 Mar 2021 17:58:41 -0400 From: =?utf-8?Q?Ludovic_Court=C3=A8s?= To: Christopher Baines Subject: Re: bug#47288: [PATCH] guix: http-client: Tweak http-multiple-get error handling. References: <20210321004338.31867-1-mail@cbaines.net> <20210325110316.862-1-mail@cbaines.net> <87r1k250za.fsf_-_@gnu.org> <87h7ky9ulu.fsf@cbaines.net> <87mtuozfep.fsf_-_@gnu.org> Date: Sat, 27 Mar 2021 22:58:39 +0100 In-Reply-To: <87mtuozfep.fsf_-_@gnu.org> ("Ludovic =?utf-8?Q?Court=C3=A8s?= =?utf-8?Q?=22's?= message of "Sat, 27 Mar 2021 18:15:42 +0100") Message-ID: <87mtuoxnqo.fsf_-_@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 47283-done Cc: 47283-done@debbugs.gnu.org, 47288-done@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) Hi! I went ahead and pushed 45fce38fb0b6c6796906149ade145b8d3594c1c6 along these lines. =E2=80=98guix weather=E2=80=99 runs to completion and things seem to work f= ine. Let me know if you notice anything wrong. Thank you for your work and for your patience on this issue! Ludo=E2=80=99. From unknown Wed Jun 18 23:13:32 2025 Received: (at fakecontrol) by fakecontrolmessage; To: internal_control@debbugs.gnu.org From: Debbugs Internal Request Subject: Internal Control Message-Id: bug archived. Date: Sun, 25 Apr 2021 11:24:06 +0000 User-Agent: Fakemail v42.6.9 # This is a fake control message. # # The action: # bug archived. thanks # This fakemail brought to you by your local debbugs # administrator