Use connection pools when requesting images from YouTube #4326
Conversation
|
Can't test as I have an external proxy for that. But sounds good |
| return pool | ||
| else | ||
| LOGGER.info("ytimg_pool: Creating a new HTTP pool for \"https://#{subdomain}.ytimg.com\"") | ||
| pool = YoutubeConnectionPool.new(URI.parse("https://#{subdomain}.ytimg.com"), capacity: CONFIG.pool_size) |
There was a problem hiding this comment.
I'm concerned about keeping that many open connections on small instances. Maybe we could bypass the pooling system if CONFIG.pool_size == 0?
There was a problem hiding this comment.
Does the pool not automatically clean up connections?
There was a problem hiding this comment.
I don't think so? It seems that released connections are pushed to the idle list, but I'm not sure what causes them to be closed:
https://github.com/crystal-lang/crystal-db/blob/3eaac85a5d4b7bee565b55dcb584e84e29fc5567/src/db/pool.cr#L151-L185
There was a problem hiding this comment.
Well that's a problem...
Regarding the bypass though, shouldn't that already be what DB:Pool does when pool_size equals 0? Invidious seems to be able to handle requests fine when CONFIG.pool_size is set to zero at least
|
I wonder if something similar to |
Regression from iv-org#2364
It would have to be somewhat different. There are so many video servers that keeping connections pools to every single one would be wasteful. But keeping the connection open per client (= almost for each video) would be nice, though I have no idea how to do that efficiently.... |
Theoretically this should improve memory usage and performance by quite a bit as we aren't creating a new
HTTP::Clientand in a turn a new connection for every image we request from YouTube.This should be tested extensively, especially in IPv6 only environment.
Closes #4009