refactor: Replace connection pool #138
Conversation
|
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh-blobs/pr/138/docs/iroh_blobs/ Last updated: 2025-08-22T07:22:44Z |
There was a problem hiding this comment.
I should've read this before reading the iroh-experiments code 🙃
Makes sense that we need a connection pool when we have multiprovider blob imports. You don't ask for a blob from a specific node after all 🙃
We need to pool connections for all kinds of reasons I guess. Sometimes it's performance and socket limitations as in HTTP2, sometimes it's because it's an abstraction boundary. Might be interesting to look into how browsers deal with HTTP3. How long do they keep the connection open, in case some JS decides to do further requests to the server? Anyhow.
LGTM, besides small nits.
…e can make util public. The ConnPool will probably move somewhere else in the longer term.
| @@ -714,6 +718,73 @@ impl TryFrom<VarInt> for Closed { | |||
| } | |||
| } | |||
|
|
|||
| pub trait ChunkRangesExt { | |||
There was a problem hiding this comment.
I guess this thing should move to bao-tree at some point!
Description
Replaces the somewhat hackish connection pool with the one from n0-computer/iroh-experiments#36 that was battle tested more.
Breaking Changes
None
Notes & open questions
Q: Expose the conn pool here?
Note: There is a nice list of possible extensions, but I think it is probably best to first get the basic version in.
Extensions would be
async fn ban(node_id: NodeId, duration: Option<Duration>), bans the node for a time, or for as long as the conn pool lives if duration is set to None.A way to observe pool stats so users know when to schedule new downloads without having to try.
Change checklist