[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#48556] [PATCH 0/4] Add keep-alive support to guix publish.
From: |
Ludovic Courtès |
Subject: |
[bug#48556] [PATCH 0/4] Add keep-alive support to guix publish. |
Date: |
Sat, 29 May 2021 17:29:28 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) |
Hi!
Great that you’re fixing this.
Mathieu Othacehe <othacehe@gnu.org> skribis:
> The default Guile web server implementation supports the keep alive
> mechanism. However, in our custom http-write implementation, the connection
> is unconditionally close after sending NAR files.
>
> To prevent that, when supported, add the client port to the server poll set so
> that further requests can be handled without closing the connection.
>
> * guix/scripts/publish.scm (nar-response-port): Duplicate the response port.
> (http-write): Add keep-alive support when sending NAR files.
Nitpick: you can use just “publish:” as the prefix in the subject line,
and s/NAR/nar/. :-)
Does this patch alone work? It seems to me that all 4 patches are
needed to actually get keep-alive support, no?
Also, this is specifically about keep-alive support for nar requests,
right? My understanding is that other requests (narinfo in particular)
already benefit from keep-alive support in (web server).
> (define (nar-response-port response compression)
> "Return a port on which to write the body of RESPONSE, the response of a
> /nar request, according to COMPRESSION."
> - (match compression
> - (($ <compression> 'gzip level)
> - ;; Note: We cannot used chunked encoding here because
> - ;; 'make-gzip-output-port' wants a file port.
> - (make-gzip-output-port (response-port response)
> - #:level level
> - #:buffer-size %default-buffer-size))
> - (($ <compression> 'lzip level)
> - (make-lzip-output-port (response-port response)
> - #:level level))
> - (($ <compression> 'zstd level)
> - (make-zstd-output-port (response-port response)
> - #:level level))
> - (($ <compression> 'none)
> - (response-port response))
> - (#f
> - (response-port response))))
> + ;; Duplicate the response port, so that it is not automatically closed when
> + ;; closing the returned port. This is needed for the keep-alive mechanism.
> + (let ((port (duplicate-port
> + (response-port response) "w+0b")))
Maybe it would be clearer if this were done at the call site, in
‘http-write’, where we can see the ‘close-port’ call a few lines below.
Otherwise LGTM.
Did you have a chance to try out the whole patch series “in production”?
It would be nice to try it out so we can catch any issues we might have
overlooked (random I/O errors, FD leaks, etc.)
Thanks,
Ludo’.