[bug#39873] [PATCH 0/3] http-multiple-get enhancements

[bug#39873] [PATCH 0/3] http-multiple-get enhancements

[Christopher Baines]

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


These are a few changes I made to the copy of http-multiple-get in the
Guix Data Service.


Christopher Baines (3):
  substitute: Use the same port for multiple request batches.
  substitute: Make http-multiple-get batch size configurable.
  substitute: Close port at the end of http-multiple-get.

 guix/scripts/substitute.scm | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 962 bytes --]

[bug#39873] [PATCH 1/3] substitute: Use the same port for multiple request batches.

[Christopher Baines]

In http-multiple-get.

* guix/scripts/substitute.scm (http-multiple-get): Switch port to p in one
occurrence.
---
 guix/scripts/substitute.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/guix/scripts/substitute.scm b/guix/scripts/substitute.scm
index dfb975a24a..5ed43d36c9 100755
--- a/guix/scripts/substitute.scm
+++ b/guix/scripts/substitute.scm
@@ -545,7 +545,7 @@ initial connection on which HTTP requests are sent."
              (()
               (reverse result))
              (remainder
-              (connect port remainder result))))
+              (connect p remainder result))))
           ((head tail ...)
            (let* ((resp   (read-response p))
                   (body   (response-body-port resp))
-- 
2.25.0

[bug#39873] [PATCH 2/3] substitute: Make http-multiple-get batch size configurable.

[Christopher Baines]

* guix/scripts/substitute.scm (http-multiple-get): Add batch-size parameter.
---
 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 5ed43d36c9..a88cb5bcfe 100755
--- a/guix/scripts/substitute.scm
+++ b/guix/scripts/substitute.scm
@@ -501,7 +501,8 @@ MAX-LENGTH first elements."
            (loop (+ 1 len) tail (cons head result)))))))
 
 (define* (http-multiple-get base-uri proc seed requests
-                            #:key port (verify-certificate? #t))
+                            #:key port (verify-certificate? #t)
+                            (batch-size 1000))
   "Send all of REQUESTS to the server at BASE-URI.  Call PROC for each
 response, passing it the request object, the response, a port from which to
 read the response body, and the previous result, starting with SEED, à la
@@ -511,7 +512,7 @@ initial connection on which HTTP requests are sent."
                 (requests requests)
                 (result   seed))
     (define batch
-      (at-most 1000 requests))
+      (at-most batch-size requests))
 
     ;; (format (current-error-port) "connecting (~a requests left)..."
     ;;         (length requests))
-- 
2.25.0

[bug#39873] [PATCH 2/3] substitute: Make http-multiple-get batch size configurable.

[Ludovic Courtès]

Christopher Baines <mail@cbaines.net> skribis:

> * guix/scripts/substitute.scm (http-multiple-get): Add batch-size parameter.

LGTM!

[bug#39873] [PATCH 3/3] substitute: Close port at the end of http-multiple-get.

[Christopher Baines]

* guix/scripts/substitute.scm (http-multiple-get): Add close-port call.
---
 guix/scripts/substitute.scm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/guix/scripts/substitute.scm b/guix/scripts/substitute.scm
index a88cb5bcfe..e3f5837a8e 100755
--- a/guix/scripts/substitute.scm
+++ b/guix/scripts/substitute.scm
@@ -544,6 +544,7 @@ initial connection on which HTTP requests are sent."
           (()
            (match (drop requests processed)
              (()
+              (close-port p)
               (reverse result))
              (remainder
               (connect p remainder result))))
-- 
2.25.0

[bug#39873] [PATCH 3/3] substitute: Close port at the end of http-multiple-get.

[Ludovic Courtès]

Christopher Baines <mail@cbaines.net> scribes:

> * guix/scripts/substitute.scm (http-multiple-get): Add close-port call.
> ---
>  guix/scripts/substitute.scm | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/guix/scripts/substitute.scm b/guix/scripts/substitute.scm
> index a88cb5bcfe..e3f5837a8e 100755
> --- a/guix/scripts/substitute.scm
> +++ b/guix/scripts/substitute.scm
> @@ -544,6 +544,7 @@ initial connection on which HTTP requests are sent."
>            (()
>             (match (drop requests processed)
>               (()
> +              (close-port p)

LGTM!

Did you notice a file descriptor leak somewhere?

Thank you!

Ludo’.

[bug#39873] [PATCH 3/3] substitute: Close port at the end of http-multiple-get.

[Christopher Baines]

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


Ludovic Courtès <ludo@gnu.org> writes:

> Christopher Baines <mail@cbaines.net> scribes:
>
>> * guix/scripts/substitute.scm (http-multiple-get): Add close-port call.
>> ---
>>  guix/scripts/substitute.scm | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/guix/scripts/substitute.scm b/guix/scripts/substitute.scm
>> index a88cb5bcfe..e3f5837a8e 100755
>> --- a/guix/scripts/substitute.scm
>> +++ b/guix/scripts/substitute.scm
>> @@ -544,6 +544,7 @@ initial connection on which HTTP requests are sent."
>>            (()
>>             (match (drop requests processed)
>>               (()
>> +              (close-port p)
>
> LGTM!
>
> Did you notice a file descriptor leak somewhere?

No, I was looking in to some wierd TLS related errors I had when using
http-multiple-get to query Curiass from the Guix Data Service, and I
noticed that maybe the port wasn't being closed when it should be.

I still haven't done a lot of testing, but I think some of these changes
have helped (although probably not this one).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 962 bytes --]

[bug#39873] [PATCH 1/3] substitute: Use the same port for multiple request batches.

[Ludovic Courtès]

Hi Christopher!

Christopher Baines <mail@cbaines.net> skribis:

> In http-multiple-get.

Leftover?  :-)

> * guix/scripts/substitute.scm (http-multiple-get): Switch port to p in one
> occurrence.
> ---
>  guix/scripts/substitute.scm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/guix/scripts/substitute.scm b/guix/scripts/substitute.scm
> index dfb975a24a..5ed43d36c9 100755
> --- a/guix/scripts/substitute.scm
> +++ b/guix/scripts/substitute.scm
> @@ -545,7 +545,7 @@ initial connection on which HTTP requests are sent."
>               (()
>                (reverse result))
>               (remainder
> -              (connect port remainder result))))
> +              (connect p remainder result))))

LGTM!

Did you notice an occurrence of this bug somewhere?

Ludo’.

[bug#39873] [PATCH 1/3] substitute: Use the same port for multiple request batches.

[Christopher Baines]

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


Ludovic Courtès <ludo@gnu.org> writes:

> Hi Christopher!
>
> Christopher Baines <mail@cbaines.net> skribis:
>
>> In http-multiple-get.
>
> Leftover?  :-)

No, just clarifying what function is being referred to. Although the
line below reveals that also.

>> * guix/scripts/substitute.scm (http-multiple-get): Switch port to p in one
>> occurrence.
>> ---
>>  guix/scripts/substitute.scm | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/guix/scripts/substitute.scm b/guix/scripts/substitute.scm
>> index dfb975a24a..5ed43d36c9 100755
>> --- a/guix/scripts/substitute.scm
>> +++ b/guix/scripts/substitute.scm
>> @@ -545,7 +545,7 @@ initial connection on which HTTP requests are sent."
>>               (()
>>                (reverse result))
>>               (remainder
>> -              (connect port remainder result))))
>> +              (connect p remainder result))))
>
> LGTM!
>
> Did you notice an occurrence of this bug somewhere?

I was looking for something that could be going wrong here as I was
seeing some kind of error talking to Cuirass using this in the Guix Data
Service, and happened to notice this in the code.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 962 bytes --]

bug#39873: [PATCH 0/3] http-multiple-get enhancements

[Christopher Baines]

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


Christopher Baines <mail@cbaines.net> writes:

> These are a few changes I made to the copy of http-multiple-get in the
> Guix Data Service.
>
>
> Christopher Baines (3):
>   substitute: Use the same port for multiple request batches.
>   substitute: Make http-multiple-get batch size configurable.
>   substitute: Close port at the end of http-multiple-get.
>
>  guix/scripts/substitute.scm | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)

I've finally got around to pushing these as
928dc1bb1c1e96e6dfbe03dac2185ecf41a7b4f5 now.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 962 bytes --]