Page MenuHomeFreeBSD

libfetch: add support for keep-alive
AbandonedPublic

Authored by bapt on Feb 18 2020, 9:02 AM.
Tags
None
Referenced Files
Unknown Object (File)
Dec 23 2023, 2:47 AM
Unknown Object (File)
Nov 6 2023, 11:12 AM
Unknown Object (File)
Nov 6 2023, 12:42 AM
Unknown Object (File)
Nov 4 2023, 7:09 PM
Unknown Object (File)
Nov 1 2023, 3:35 AM
Unknown Object (File)
Oct 5 2023, 10:09 AM
Unknown Object (File)
Oct 3 2023, 7:09 PM
Unknown Object (File)
Oct 1 2023, 3:38 AM
Subscribers

Details

Reviewers
emaste
kevans
bapt
Group Reviewers
manpages
Summary

Implement http keep-alive
Implementations notes:

  • the implementation is heavily based on xbps package manager bundled

version of libfetch.

  • the implementation isn't thread safe

if the cache are not initialize libfetch will not request a keep-alive connection
a fetch_connect2 has been implemented to not break the ABI by modifying fetch_connect()

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 29475
Build 27348: arc lint + arc unit

Event Timeline

bcr added a subscriber: bcr.

OK from manpages.

kevans added inline comments.
lib/libfetch/common.c
1836

I think the latter 2/3 of this chain would be more succinctly written else cache_global_limit = MAX(per_host_limit, global_limit), but I won't insist. =-)

This revision is now accepted and ready to land.Feb 18 2020, 6:23 PM

Simplify the code to compute the global cache limits
per @kevans request

This revision now requires review to proceed.Feb 18 2020, 8:06 PM
bapt marked an inline comment as done.
This revision is now accepted and ready to land.Feb 18 2020, 8:07 PM
jilles added inline comments.
lib/libfetch/common.c
141–142

These don't seem to follow libfetch's naming convention for exported symbols. Consider not exporting them via __hidden and moving their declaration to a header file that is not installed such as common.h.

1842

Given that libfetch is not thread-safe, most of the time the best per_host_limit is going to be 1.

lib/libfetch/common.h
126

So applications cannot use fetch_connect() by including <fetch.h> but can if they declare it manually (since libfetch neither has a version script nor __hidden annotations nor compiler options to control visibility). It is unclear whether this is "part of the ABI".

lib/libfetch/http.c
1626

It may be helpful to do something about cached connections that have been closed by the server, for example:

  • For idempotent requests (GET/HEAD/PUT/DELETE), transparently retry on a new connection if there is a network error or connection closure before anything is received on a cached connection.
  • For other requests, do not use cached connection at all (but it is OK to cache the connection for later use by an idempotent request).
  • Close cached connections that have been idle for too long (it is good enough to do this only when otherwise touching the cache; no need to spin up a thread).
1854–1855

Connection: keep-alive is a HTTP/1.0 thing; with HTTP/1.1, keepalive is default.

So I suppose the check should be that the HTTP version must be at least 1.1 and there is no close token in the Connection header; I don't think it is worth spending any effort to make keepalive work on HTTP/1.0.