[PATCH 0/3] http-multiple-get enhancements

  • Done
  • quality assurance status badge
Details
2 participants
  • Ludovic Courtès
  • Christopher Baines
Owner
unassigned
Submitted by
Christopher Baines
Severity
normal

Debbugs page

Christopher Baines wrote 5 years ago
(address . guix-patches@gnu.org)
87imjmeagu.fsf@cbaines.net
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(-)
-----BEGIN PGP SIGNATURE-----

iQKTBAEBCgB9FiEEPonu50WOcg2XVOCyXiijOwuE9XcFAl5ddIFfFIAAAAAALgAo
aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF
ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcACgkQXiijOwuE
9XecExAAhLXSlDod7wFD7C1Hpf6N1vk/YYPpjhe+xFVWZtGZ069ZZ0JCZ45R1qKF
F3C3SOPpMSeiTHepRqUP0eoNkDugH1nOMZRYymFdeUq4iSTuntYTw5juLeBFFeP9
AUerXp3qMEX0rkV2/uwvIrSHT3FRmVHRdsP7RVEcI+eDVvV80QxzMucQJqQmm5bB
GSU7M9Vwp/RL8+SZfEZpbGwCJ734O1BIOd3cExISG81/JPPD0W+GFK9fZKwt3wbL
dsXq4B6kH+S/bK0mTQhVSiQNS3YSjRyMtQPPkdgvw3Y1MH5P2yrIXufWq8MH7aiZ
KFmSlu87SZh+o9FXrq4esmDszC6XC/Jegexvm1eCTCUmW+g65h092+4DTkrfTjaL
hARBDyKMB6w77uy69sryM2K6c6Tlddz+7LPRLwQ1r8plLbNqd17V5ySbjuUISoPs
uezJcNN9lgztlkKHxLh371lvYgiKnye1ky+IzJ8UKlXMW/Hz0/aNJR4mvk0SXeuQ
w1tpM166aVFj4ui03r03eenx0Vaeq2uE7Ug/ebxCojjZ7BUJDYIGrPNs/qRpSCNr
zE3uVcCsdre2pWsQPrW3HaK+gnHyD3DxvrAMOn9ZIA03e68wKgwXmlVW6HKn7W79
A0BW7l5EJVfK13Eyx5UR2MYYboUql9fLwBPWD8IsY5T/u8bWmdg=
=MglG
-----END PGP SIGNATURE-----

Christopher Baines wrote 5 years ago
[PATCH 1/3] substitute: Use the same port for multiple request batches.
(address . 39873@debbugs.gnu.org)
20200302210735.13337-1-mail@cbaines.net
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(-)

Toggle diff (15 lines)
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
Christopher Baines wrote 5 years ago
[PATCH 2/3] substitute: Make http-multiple-get batch size configurable.
(address . 39873@debbugs.gnu.org)
20200302210735.13337-2-mail@cbaines.net
* guix/scripts/substitute.scm (http-multiple-get): Add batch-size parameter.
---
guix/scripts/substitute.scm | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

Toggle diff (25 lines)
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
Christopher Baines wrote 5 years ago
[PATCH 3/3] substitute: Close port at the end of http-multiple-get.
(address . 39873@debbugs.gnu.org)
20200302210735.13337-3-mail@cbaines.net
* guix/scripts/substitute.scm (http-multiple-get): Add close-port call.
---
guix/scripts/substitute.scm | 1 +
1 file changed, 1 insertion(+)

Toggle diff (14 lines)
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
Ludovic Courtès wrote 5 years ago
Re: [bug#39873] [PATCH 1/3] substitute: Use the same port for multiple request batches.
(name . Christopher Baines)(address . mail@cbaines.net)(address . 39873@debbugs.gnu.org)
875zffg97p.fsf@gnu.org
Hi Christopher!

Christopher Baines <mail@cbaines.net> skribis:

Toggle quote (2 lines)
> In http-multiple-get.

Leftover? :-)

Toggle quote (17 lines)
> * 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’.
Ludovic Courtès wrote 5 years ago
Re: [bug#39873] [PATCH 2/3] substitute: Make http-multiple-get batch size configurable.
(name . Christopher Baines)(address . mail@cbaines.net)(address . 39873@debbugs.gnu.org)
871rq3g97f.fsf@gnu.org
Christopher Baines <mail@cbaines.net> skribis:

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

LGTM!
Ludovic Courtès wrote 5 years ago
Re: [bug#39873] [PATCH 3/3] substitute: Close port at the end of http-multiple-get.
(name . Christopher Baines)(address . mail@cbaines.net)(address . 39873@debbugs.gnu.org)
87wo7veulg.fsf@gnu.org
Christopher Baines <mail@cbaines.net> scribes:

Toggle quote (15 lines)
> * 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’.
Christopher Baines wrote 5 years ago
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 39873@debbugs.gnu.org)
877dzue6v8.fsf@cbaines.net
Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (21 lines)
> 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).
-----BEGIN PGP SIGNATURE-----

iQKTBAEBCgB9FiEEPonu50WOcg2XVOCyXiijOwuE9XcFAl5lhrtfFIAAAAAALgAo
aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF
ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcACgkQXiijOwuE
9XemRA//XN+hsKKOfddNAzOdb8xe6VzkqEIVjhgensZmBQuskx+iXqyMzXGy9YFG
Q+PdhmXQZQzREgy/w8Ua0c8APMRzbulKpBloXnxGt+vnu3HPuDEcdvcTynmt1YHB
yGXesfb0izj9JTbg9pyQDx4tukvSlrnat/NWy+C5EtS6HXc0BZeCBz/jMpoj7rv/
z47YxnrJ0MGt2KR0hEkyd53w7QBJUDrS7KuqCii3rfkJg6uKAs+5OAw98ZYC0Emb
uri/nb1y8EnQaVzJHrKS5nHlh11szj3dMJ06JN/lLynUcX0EQzlmd9tdVRPMP4nN
D/Lo3p8Vmc/etL1CE8M/EfYPSPa8jxe1KvWzLovpny2Z1l0NQZumgEprr6M+XH6L
k9Holo3EegcdMun9aggaXTqA8SAKSaR+Nls/sjb1nPV6iZV4MQKsQ7P1Co7h/OZt
s5qmwrzotdAO6GuVDTL6wLHQvAGOKHKiHgbTWAjatKXRbsAjNXnyPG41L7Dr/2Ep
wYYwXUE6tpeYq1f056m7R2LzZVQS8gpw70azF83/LoYYSOV5rV4unnBeqoaWmXTJ
9OCgr+HbEOquAIdvtWb36Yb2eNImrxoplwlN6DyUpn1IZcCcRXAEoEaF2pX5OEDw
e6tOB5WY1XADBCzAnCXff0YAPeSOgsUAB49SQBa+Ul7bzznhFpA=
=eviw
-----END PGP SIGNATURE-----

Christopher Baines wrote 5 years ago
Re: [bug#39873] [PATCH 1/3] substitute: Use the same port for multiple request batches.
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 39873@debbugs.gnu.org)
875zfee6gd.fsf@cbaines.net
Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (8 lines)
> 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.

Toggle quote (21 lines)
>> * 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.
-----BEGIN PGP SIGNATURE-----

iQKTBAEBCgB9FiEEPonu50WOcg2XVOCyXiijOwuE9XcFAl5liNJfFIAAAAAALgAo
aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF
ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcACgkQXiijOwuE
9XeJvg/+Pb3lzcFhNPDt83Fs6nDeLv9DGRCD9eRHyk6AHj0c1MtXwY43ezcF4ZJm
gAQAxgfTYVJMJMes7rXmXtzxPZroneHjGymFFCpR8iqM2umC1cwR8LLvHufSU6tq
/ACB9oM5TK7WCrQodffua8mOq0Y4uLWk8g0bJycQo+w53Grsp2FSPjLyDcGtyxLW
bm0iM5cz82QeFH/1N/Vmrr2YSRMN1K45Jd9UE311qEEMp9pkNQI3TF58XBuCpCyw
5vaztgtZDLg1zEdIVFokJjkFrmd/Tm7BEqhXO9U6wulL+Q0j+eo4bOGFMMoG/Y80
8utWzb+9sQqIpiHX242fo5+p1iwofuJbb5/Ef9CVwD37sK2QoeiAezz34U2s8XJF
lVhd4JRxY6vDP106HNWN9htPQFR1VtGmu2tF7TlJi3O3vBVNq1+B4+CCbfhXhJm5
GHCvJHyRVLV1sLdFDQgc6SKwfAUtMVchKrNs2j7IrreDPDOX9DofD5iqqXNF1kta
ZCeuLdU8Ytvx41jp8+PQw3heK/LJAhm0lmYbVlcRYRmARZEGzvaknbH/v7H84PC+
zt+kWPqC47Xva6GneyYfpD6iuaF3uvfYBdyPqZPGxZVVF4evxWHARRrJFvokhpcd
TD02u5iLv67LpgzSUDvbATTQIO8NJcTPwrQKrHpWdnl3rFXvhEs=
=fWO2
-----END PGP SIGNATURE-----

Christopher Baines wrote 5 years ago
Re: [bug#39873] [PATCH 0/3] http-multiple-get enhancements
(address . 39873-done@debbugs.gnu.org)
874kt6nl98.fsf@cbaines.net
Christopher Baines <mail@cbaines.net> writes:

Toggle quote (12 lines)
> 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.
-----BEGIN PGP SIGNATURE-----

iQKTBAEBCgB9FiEEPonu50WOcg2XVOCyXiijOwuE9XcFAl6l1aNfFIAAAAAALgAo
aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF
ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcACgkQXiijOwuE
9Xf5hRAAif5j8hWtfIfr8x02rZ5U4sNAHh2yR9cpY9v1Z88gQtxcbKk77YaSnEic
YSW+gDo7lshUeiyusexjLFmjUepZSeo7HmmE/agt/Bopgdm1jXN0jkr4toQddWuX
nwBTW07RnH+M+gt0sdoMDfBEEIxVj8zCEUK+DlK5X0Ta38aClm317wN0Q2XcLvcK
z3i0mrO/4NQHf335jqjldn9mswhuybjA2HS1eEf+zSfmtLU1pr/qtoMnR6T5L63k
pCXqH7gLx0RyMdv8rV+fuUAqBQy7RDZ3EjnUSzWuDn/Ej+jzxNjNb1kZgs0Lxoau
9vfK5LyzIWD9wRCXzGd6GhclJx863XRFYsff93wsTR2n6wk2xVIWVT8hx2+HPISr
kUeTimQC0O862dNGFOTs4eLIVeiBJveeLeyW3tMjIy8wNK7kwTrJS7LtuXllGaXl
Ea7dxUPYFeZZc93rS/wmyRbZC6ES+z2yvLfzhIggD5HjbFLHDvT+V2is68hyudR8
3/16go//7EfzVJARMB5XqGx7zhG9IeApE66Ux4Jhu4j9CVsqtYD0z6DMg5E9LjPH
upVCqBW41QO4LWxZdNUdLINBgAk1DojzSKZj/8xlcjy609R0pg/wUz4nDV9GRqlK
7/kB+Cbo6qQu39qM967aXjeDLoLx3CwEqRj4fNh21lTb+mrr0wo=
=E2dk
-----END PGP SIGNATURE-----

Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 39873@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 39873
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch
You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags
mumi command -t +confirmed -t +easy
Or, remove the moreinfo tag and set the help tag
mumi command -t -moreinfo -t +help