[bug#48556] [PATCH 0/4] Add keep-alive support to guix publish.

[Ludovic Courtès]

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’.