Feature request: better TCP connection pool implementation
- Dominant language
- C++
- Stars
- 60
- Forks
- 100
- Avg merge
- 1h 50m
- Merged PRs (30d)
- 25
Description
Hi team, I'm trying understand the connection pool logic for the extension. We have a connection pool based on the assumption that the extension leverages HTTP/1.1, and per-client represent a TCP connection.
In detail, connections and http clients are cached here: https://github.com/duckdb/duckdb-httpfs/blob/20403b967d3ac13a833a63b943b680f76c0307e0/src/include/httpfs.hpp#L32-L46
But I'm wondering if there're a few things that we miss:
- We don't see eviction logic for the cache, there're several factors we could potentially improve, for example, cap max number of TCP connections and evict based on LRU, evict cached HTTP clients based on idle timeout, etc
+ Excessive TCP connection number hurts performance
- Now we're using libcurl as network backend, which allows us easily switch to HTTP/2
+ reference: https://curl.se/libcurl/c/CURLOPT_HTTP_VERSION.html
+ For HTTP/2, due to multiplexing, the connection pool logic is completely different, usually there're two ways to maintain the pool
+ either we pre-create multiple connections (libcurl support: https://curl.se/libcurl/c/CURLMOPT_MAX_HOST_CONNECTIONS.html) and round-robin
+ or new connections are created when ongoing request number (aka, active HTTP/2 stream number) exceeds certain threshold (by default, HTTP/2 supports at max 100 concurrent streams)
I'm wondering if it's possible to adopt a more decoupled design, so connection pool management and HTTP client could be managed separately, and better support for different HTTP protocols. I'm happy to collaborate if there's anything I could help. :)
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with the connection and HTTP client cache in src/include/httpfs.hpp at lines 32-46, then review the linked libcurl HTTP version and connection-limit documentation. Clarify the supported HTTP protocols, eviction policy, and pooling model before implementation; done should mean an agreed design that handles connection limits and HTTP/2 behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- backend, networking
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 35/100