[PATCH] scripts: substitute: Avoid infinite looping when updating the substitute list

[PATCH] scripts: substitute: Avoid infinite looping when updating the substitute list

[Andy Patterson]

[-- Attachment #1: Type: text/plain, Size: 1016 bytes --]

Hi,

I was having trouble using substitutes, so I decided to investigate it.
I found out that when retrieving the batch of .narinfo files from the
server, my connection was always being closed after the first response.
Therefore, I would always be reconnecting to the server with every
request in the batch still to process, and this reconnecting would be
repeated infinitely, or at least until my Internet went down. Based on
the documentation for the "connection: close" header, I think that this
first response should be complete, so we can use it. Maybe I'm wrong;
I'm not totally familiar with how http works.

I also considered ensuring that a good response code was received, but
the handler deals with error response codes as well, so it seemed
superfluous.

This patch will make use of the last response from the server, before
reconnecting to try the next one. With it I was able to successfully
build and run links.

Let me know if my understanding is off, or if there's a better approach.

Thanks,

Andy

[-- Attachment #2: 0001-scripts-substitute-Avoid-infinite-looping-when-updat.patch --]
[-- Type: text/x-patch, Size: 1304 bytes --]

From 633c491e2dad4c3a70eb045177c17bd2552d63f3 Mon Sep 17 00:00:00 2001
From: Andy Patterson <ajpatter@uwaterloo.ca>
Date: Sun, 5 Jul 2015 19:34:05 -0400
Subject: [PATCH] scripts: substitute: Avoid infinite looping when updating the
 substitute list

* guix/scripts/substitute.scm (http-multiple-get): Make use of the current
  response before reconnecting.
---
 guix/scripts/substitute.scm | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/guix/scripts/substitute.scm b/guix/scripts/substitute.scm
index 8b4fa36..e767d0f 100755
--- a/guix/scripts/substitute.scm
+++ b/guix/scripts/substitute.scm
@@ -468,8 +468,9 @@ to read the response body.  Return the list of results."
              ;; case we have to try again.  Check whether that is the case.
              (match (assq 'connection (response-headers resp))
                (('connection 'close)
-                (close-port p)
-                (connect requests result))        ;try again
+                (let ((first-result (proc head resp body)))
+                  (close-port p)
+                  (connect tail (cons first-result result))))     ;try again
                (_
                 (loop tail                        ;keep going
                       (cons (proc head resp body) result)))))))))))
-- 
2.4.5

Re: [PATCH] scripts: substitute: Avoid infinite looping when updating the substitute list

[Ludovic Courtès]

Hi,

Andy Patterson <ajpatter@uwaterloo.ca> skribis:

> I was having trouble using substitutes, so I decided to investigate it.
> I found out that when retrieving the batch of .narinfo files from the
> server, my connection was always being closed after the first response.
> Therefore, I would always be reconnecting to the server with every
> request in the batch still to process, and this reconnecting would be
> repeated infinitely, or at least until my Internet went down. Based on
> the documentation for the "connection: close" header, I think that this
> first response should be complete, so we can use it. Maybe I'm wrong;
> I'm not totally familiar with how http works.

Indeed, <https://tools.ietf.org/html/rfc7230#section-6.1> reads:

  The "close" connection option is defined for a sender to signal that
  this connection will be closed after completion of the response.

> This patch will make use of the last response from the server, before
> reconnecting to try the next one. With it I was able to successfully
> build and run links.
>
> Let me know if my understanding is off, or if there's a better approach.

I think you found a genuine bug and provided the right patch!

Did you observe the problem when connecting to hydra.gnu.org, or was it
another server?  Did you have a way to reproduce it?

(In my experience Nginx at hydra.gnu.org closes the connection after
~100 responses, so even if the last response is not consumed directly,
that’s OK.)

> From 633c491e2dad4c3a70eb045177c17bd2552d63f3 Mon Sep 17 00:00:00 2001
> From: Andy Patterson <ajpatter@uwaterloo.ca>
> Date: Sun, 5 Jul 2015 19:34:05 -0400
> Subject: [PATCH] scripts: substitute: Avoid infinite looping when updating the
>  substitute list
>
> * guix/scripts/substitute.scm (http-multiple-get): Make use of the current
>   response before reconnecting.

I’ve committed a slightly edited version of the patch where I factorized
the call to PROC and changed the commit message to describe the changes
at the source code level, in GNU change log style.  (I hope this is fine
with you.)

Thank you!

Ludo’.

Re: [PATCH] scripts: substitute: Avoid infinite looping when updating the substitute list

[Andy Patterson]

Hi Ludo,

> Did you observe the problem when connecting to hydra.gnu.org, or was it
> another server?  Did you have a way to reproduce it?

I was only connecting to hydra.gnu.org, and was using a default setup.
First I used the install image, and then on my current setup with a
version built from git. I was always able to reproduce the problem. I
think it may be that the network setup where I live is causing the
connection to always be closed, as it seems that no-one else has
experienced it.

> I’ve committed a slightly edited version of the patch where I factorized
> the call to PROC and changed the commit message to describe the changes
> at the source code level, in GNU change log style.  (I hope this is fine
> with you.)

That's good, thanks! I wasn't too sure on the specifics of the GNU style
and mostly tried to imitate examples from the recent history. Seeing
what you've changed gives me a better idea of the correct style.

Cheers,

Andy

Re: [PATCH] scripts: substitute: Avoid infinite looping when updating the substitute list

[Ludovic Courtès]

Andy Patterson <ajpatter@uwaterloo.ca> skribis:

>> Did you observe the problem when connecting to hydra.gnu.org, or was it
>> another server?  Did you have a way to reproduce it?
>
> I was only connecting to hydra.gnu.org, and was using a default setup.
> First I used the install image, and then on my current setup with a
> version built from git. I was always able to reproduce the problem. I
> think it may be that the network setup where I live is causing the
> connection to always be closed, as it seems that no-one else has
> experienced it.

Yes, it could have been some special firewall settings or something like
that.

Anyway, thank you!

Ludo’.