| Message ID | 20210315192449.16248-2-mail@cbaines.net |
|---|---|
| State | New |
| Headers | show |
| Series | substitute: Handle closing connections to substitute servers. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/submitting builds | success | |
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
| cbaines/issue | success | View issue |
Christopher Baines <mail@cbaines.net> skribis: > When reusing a HTTP connection to fetch multiple nars, and the remote server > signals that the connection should be closed. > > * guix/scripts/substitute.scm (process-substitution): Close connections to > substitute servers when a Connection: close header is specified in the > response. In the context of <https://issues.guix.gnu.org/47157>, honoring “Connection: close” isn’t enough. We need to handle the case where the server didn’t express the intent to close the connection but eventually closed it after some time. Does that make sense? Ludo’.
Ludovic Courtès <ludo@gnu.org> writes: > Christopher Baines <mail@cbaines.net> skribis: > >> When reusing a HTTP connection to fetch multiple nars, and the remote server >> signals that the connection should be closed. >> >> * guix/scripts/substitute.scm (process-substitution): Close connections to >> substitute servers when a Connection: close header is specified in the >> response. > > In the context of <https://issues.guix.gnu.org/47157>, honoring > “Connection: close” isn’t enough. We need to handle the case where the > server didn’t express the intent to close the connection but eventually > closed it after some time. > > Does that make sense? Yeah, of course, this was something I was thinking about in addition to the changes in [1]. 1: https://issues.guix.gnu.org/47160
diff --git a/guix/scripts/substitute.scm b/guix/scripts/substitute.scm index cb79ea6927..deb6fbdaa2 100755 --- a/guix/scripts/substitute.scm +++ b/guix/scripts/substitute.scm @@ -406,7 +406,9 @@ the current output port." (case (uri-scheme uri) ((file) (let ((port (open-file (uri-path uri) "r0b"))) - (values port (stat:size (stat port))))) + (values port + (stat:size (stat port)) + (const #t)))) ; no cleanup to do ((http https) (guard (c ((http-get-error? c) (leave (G_ "download from '~a' failed: ~a, ~s~%") @@ -434,7 +436,12 @@ the current output port." #:buffered? #f #:verify-certificate? #f))) (values raw - (response-content-length response)))))))) + (response-content-length response) + (match (assq 'connection (response-headers response)) + (('connection 'close) + (lambda () + (close-port (response-port response)))) + (_ (const #t)))))))))) (else (leave (G_ "unsupported substitute URI scheme: ~a~%") (uri->string uri))))) @@ -449,7 +456,7 @@ the current output port." (format (current-error-port) (G_ "Downloading ~a...~%") (uri->string uri))) - (let*-values (((raw download-size) + (let*-values (((raw download-size post-fetch-cleanup) ;; 'guix publish' without '--cache' doesn't specify a ;; Content-Length, so DOWNLOAD-SIZE is #f in this case. (fetch uri)) @@ -493,6 +500,10 @@ the current output port." ;; Wait for the reporter to finish. (every (compose zero? cdr waitpid) pids) + ;; Do post-fetch cleanup, maybe closing the HTTP connection if HTTP is + ;; being used, and the connection should be closed + (post-fetch-cleanup) + ;; Skip a line after what 'progress-reporter/file' printed, and another ;; one to visually separate substitutions. (display "\n\n" (current-error-port))