From 78b7db17f4bc7b7352a2eac9dfff3ee7b8e063f8 Mon Sep 17 00:00:00 2001 From: syeopite Date: Wed, 9 Apr 2025 13:07:01 -0700 Subject: [PATCH 01/25] Add support for setting max idle http pool size --- config/config.example.yml | 15 ++++++- src/invidious.cr | 11 ++++-- src/invidious/config.cr | 6 ++- src/invidious/yt_backend/connection_pool.cr | 44 +++++++++++++++++---- 4 files changed, 63 insertions(+), 13 deletions(-) diff --git a/config/config.example.yml b/config/config.example.yml index d6119f29..781d0961 100644 --- a/config/config.example.yml +++ b/config/config.example.yml @@ -206,7 +206,7 @@ https_only: false #disable_proxy: false ## -## Size of the HTTP pool used to connect to youtube. Each +## Max size of the HTTP pool used to connect to youtube. Each ## domain ('youtube.com', 'ytimg.com', ...) has its own pool. ## ## Accepted values: a positive integer @@ -215,6 +215,19 @@ https_only: false #pool_size: 100 +## +## Idle size of the HTTP pool used to connect to youtube. Each +## domain ('youtube.com', 'ytimg.com', ...) has its own pool. +## +## When unset this value has the same value as pool_size +## +## Accepted values: a positive integer +## Default: (internally this means that it has the same value as pool_size) +## +#idle pool_size: 100 + + + ## ## Additional cookies to be sent when requesting the youtube API. ## diff --git a/src/invidious.cr b/src/invidious.cr index d3300ece..eb22e9f8 100644 --- a/src/invidious.cr +++ b/src/invidious.cr @@ -91,14 +91,19 @@ SOFTWARE = { "branch" => "#{CURRENT_BRANCH}", } -YT_POOL = YoutubeConnectionPool.new(YT_URL, capacity: CONFIG.pool_size) +YT_POOL = YoutubeConnectionPool.new(YT_URL, max_capacity: CONFIG.pool_size, idle_capacity: CONFIG.idle_pool_size) # Image request pool -GGPHT_POOL = YoutubeConnectionPool.new(URI.parse("https://yt3.ggpht.com"), capacity: CONFIG.pool_size) +GGPHT_POOL = YoutubeConnectionPool.new( + URI.parse("https://yt3.ggpht.com"), + max_capacity: CONFIG.pool_size, + idle_capacity: CONFIG.idle_pool_size +) COMPANION_POOL = CompanionConnectionPool.new( - capacity: CONFIG.pool_size + max_capacity: CONFIG.pool_size, + idle_capacity: CONFIG.idle_pool_size ) # CLI diff --git a/src/invidious/config.cr b/src/invidious/config.cr index 4d69854c..00897371 100644 --- a/src/invidious/config.cr +++ b/src/invidious/config.cr @@ -157,8 +157,12 @@ class Config property host_binding : String = "0.0.0.0" # Path and permissions to make Invidious listen on a UNIX socket instead of a TCP port property socket_binding : SocketBindingConfig? = nil - # Pool size for HTTP requests to youtube.com and ytimg.com (each domain has a separate pool of `pool_size`) + + # Max pool size for HTTP requests to youtube.com and ytimg.com (each domain has a separate pool) property pool_size : Int32 = 100 + # Idle pool size for HTTP requests to youtube.com and ytimg.com (each domain has a separate pool) + property idle_pool_size : Int32? = nil + # HTTP Proxy configuration property http_proxy : HTTPProxyConfig? = nil diff --git a/src/invidious/yt_backend/connection_pool.cr b/src/invidious/yt_backend/connection_pool.cr index 0daed46c..da6f6556 100644 --- a/src/invidious/yt_backend/connection_pool.cr +++ b/src/invidious/yt_backend/connection_pool.cr @@ -4,11 +4,24 @@ private YTIMG_POOLS = {} of String => YoutubeConnectionPool struct YoutubeConnectionPool property! url : URI - property! capacity : Int32 + property! max_capacity : Int32 + property! idle_capacity : Int32 property! timeout : Float64 property pool : DB::Pool(HTTP::Client) - def initialize(url : URI, @capacity = 5, @timeout = 5.0) + def initialize( + url : URI, + *, + @max_capacity : Int32 = 5, + idle_capacity : Int32? = nil, + @timeout : Float64 = 5.0, + ) + if idle_capacity.nil? + @idle_capacity = @max_capacity + else + @idle_capacity = idle_capacity + end + @url = url @pool = build_pool() end @@ -33,10 +46,12 @@ struct YoutubeConnectionPool end private def build_pool + # We call the getter for the instance variables instead of using them directly + # because the getters defined by property! ensures that the value is not a nil options = DB::Pool::Options.new( initial_pool_size: 0, - max_pool_size: capacity, - max_idle_pool_size: capacity, + max_pool_size: max_capacity, + max_idle_pool_size: idle_capacity, checkout_timeout: timeout ) @@ -47,13 +62,22 @@ struct YoutubeConnectionPool end struct CompanionConnectionPool + property! max_capacity : Int32 + property! idle_capacity : Int32 + property! timeout : Float64 property pool : DB::Pool(HTTP::Client) - def initialize(capacity = 5, timeout = 5.0) + def initialize(*, @max_capacity : Int32 = 5, idle_capacity : Int32? = nil, @timeout : Float64 = 5.0) + if idle_capacity.nil? + @idle_capacity = @max_capacity + else + @idle_capacity = idle_capacity + end + options = DB::Pool::Options.new( initial_pool_size: 0, - max_pool_size: capacity, - max_idle_pool_size: capacity, + max_pool_size: max_capacity, + max_idle_pool_size: idle_capacity.not_nil!, checkout_timeout: timeout ) @@ -145,7 +169,11 @@ def get_ytimg_pool(subdomain) 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) + pool = YoutubeConnectionPool.new( + URI.parse("https://#{subdomain}.ytimg.com"), + max_capacity: CONFIG.pool_size, + idle_capacity: CONFIG.idle_pool_size + ) YTIMG_POOLS[subdomain] = pool return pool From 247254b3bb4c9e17e86d09251236d5df6d534481 Mon Sep 17 00:00:00 2001 From: syeopite Date: Mon, 11 Nov 2024 15:58:35 -0800 Subject: [PATCH 02/25] Refactor connection pooling logic - Remove duplication between standard and companion pool - Raises a wrapped exception on any DB:Error - Don't use a non-pool client when client fails - Ensure that client is always released - Add documentation to various pool methods --- src/invidious.cr | 7 +- src/invidious/connection/pool.cr | 116 ++++++++++++++++++++ src/invidious/yt_backend/connection_pool.cr | 111 +------------------ 3 files changed, 123 insertions(+), 111 deletions(-) create mode 100644 src/invidious/connection/pool.cr diff --git a/src/invidious.cr b/src/invidious.cr index eb22e9f8..ac9fa4b1 100644 --- a/src/invidious.cr +++ b/src/invidious.cr @@ -35,6 +35,7 @@ require "protodec/utils" require "./invidious/database/*" require "./invidious/database/migrations/*" +require "./invidious/connection/*" require "./invidious/http_server/*" require "./invidious/helpers/*" require "./invidious/yt_backend/*" @@ -91,17 +92,17 @@ SOFTWARE = { "branch" => "#{CURRENT_BRANCH}", } -YT_POOL = YoutubeConnectionPool.new(YT_URL, max_capacity: CONFIG.pool_size, idle_capacity: CONFIG.idle_pool_size) +YT_POOL = Invidious::ConnectionPool::Pool.new(YT_URL, max_capacity: CONFIG.pool_size, idle_capacity: CONFIG.idle_pool_size) # Image request pool -GGPHT_POOL = YoutubeConnectionPool.new( +GGPHT_POOL = Invidious::ConnectionPool::Pool.new( URI.parse("https://yt3.ggpht.com"), max_capacity: CONFIG.pool_size, idle_capacity: CONFIG.idle_pool_size ) -COMPANION_POOL = CompanionConnectionPool.new( +COMPANION_POOL = Invidious::ConnectionPool::CompanionPool.new( max_capacity: CONFIG.pool_size, idle_capacity: CONFIG.idle_pool_size ) diff --git a/src/invidious/connection/pool.cr b/src/invidious/connection/pool.cr new file mode 100644 index 00000000..ee69eb40 --- /dev/null +++ b/src/invidious/connection/pool.cr @@ -0,0 +1,116 @@ +module Invidious::ConnectionPool + # The base connection pool that provides the underlying logic that all connection pools are based around + # + # Uses `DB::Pool` for the pooling logic + abstract struct BaseConnectionPool(PoolClient) + # Returns the max size of the connection pool + getter max_capacity : Int32 + + # Returns the configured checkout time out + getter timeout : Float64 + + # Creates a connection pool with the provided options + def initialize( + @max_capacity : Int32 = 5, + @idle_capacity : Int32? = nil, + @timeout : Float64 = 5.0 + ) + @pool = build_pool() + end + + # Returns the idle capacity for the connection pool; if unset this is the same as `max_capacity`. + # + # This means that when idle capacity is unset the pool will keep all connections around forever, all the + # way until it reaches max capacity. + def idle_capacity : Int32 + if (idle = @idle_capacity).nil? + return @max_capacity + end + + return idle + end + + # Returns the underlying `DB::Pool` object + abstract def pool : DB::Pool(PoolClient) + + # Checks out a client from the pool + def client(&) + pool.checkout do |http_client| + # Proxy needs to be reinstated every time we get a client from the pool + http_client.proxy = make_configured_http_proxy_client() if CONFIG.http_proxy + + response = yield http_client + + return response + rescue ex : DB::Error + # Prevent broken client from being checked back into the pool + http_client.close + raise ConnectionPool::Error.new(ex.message, cause: ex) + ensure + pool.release(http_client) + end + rescue ex : DB::PoolTimeout + # Failed to checkout a client + raise ConnectionPool::Error.new(ex.message, cause: ex) + end + + # Builds a connection pool + private abstract def build_pool : DB::Pool(PoolClient) + + # Creates a `DB::Pool::Options` used for constructing `DB::Pool` + private def pool_options : DB::Pool::Options + return DB::Pool::Options.new( + initial_pool_size: 0, + max_pool_size: max_capacity, + max_idle_pool_size: idle_capacity, + checkout_timeout: timeout + ) + end + end + + # A basic connection pool where each client within is set to connect to a single resource + struct Pool < BaseConnectionPool(HTTP::Client) + # The url each client within the pool will connect to + getter url : URI + getter pool : DB::Pool(HTTP::Client) + + # Creates a pool of clients that connects to the given url, with the provided options. + def initialize( + url : URI, + *, + @max_capacity : Int32 = 5, + @idle_capacity : Int32? = nil, + @timeout : Float64 = 5.0 + ) + + @url = url + @pool = build_pool() + end + + # :inherit: + private def build_pool : DB::Pool(HTTP::Client) + return DB::Pool(HTTP::Client).new(pool_options) do + make_client(url, force_resolve: true) + end + end + end + + # A modified connection pool for the interacting with Invidious companion. + # + # The main difference is that clients in this pool are created with different urls + # based on what is randomly selected from the configured list of companions + struct CompanionPool < BaseConnectionPool(HTTP::Client) + getter pool : DB::Pool(HTTP::Client) + + # :inherit: + private def build_pool : DB::Pool(HTTP::Client) + return DB::Pool(HTTP::Client).new(pool_options) do + companion = CONFIG.invidious_companion.sample + make_client(companion.private_url, use_http_proxy: false) + end + end + end + + class Error < Exception + end +end diff --git a/src/invidious/yt_backend/connection_pool.cr b/src/invidious/yt_backend/connection_pool.cr index da6f6556..512a26f1 100644 --- a/src/invidious/yt_backend/connection_pool.cr +++ b/src/invidious/yt_backend/connection_pool.cr @@ -1,111 +1,6 @@ -# Mapping of subdomain => YoutubeConnectionPool +# Mapping of subdomain => Invidious::ConnectionPool::Pool # This is needed as we may need to access arbitrary subdomains of ytimg -private YTIMG_POOLS = {} of String => YoutubeConnectionPool - -struct YoutubeConnectionPool - property! url : URI - property! max_capacity : Int32 - property! idle_capacity : Int32 - property! timeout : Float64 - property pool : DB::Pool(HTTP::Client) - - def initialize( - url : URI, - *, - @max_capacity : Int32 = 5, - idle_capacity : Int32? = nil, - @timeout : Float64 = 5.0, - ) - if idle_capacity.nil? - @idle_capacity = @max_capacity - else - @idle_capacity = idle_capacity - end - - @url = url - @pool = build_pool() - end - - def client(&) - conn = pool.checkout - # Proxy needs to be reinstated every time we get a client from the pool - conn.proxy = make_configured_http_proxy_client() if CONFIG.http_proxy - - begin - response = yield conn - rescue ex - conn.close - conn = make_client(url, force_resolve: true) - - response = yield conn - ensure - pool.release(conn) - end - - response - end - - private def build_pool - # We call the getter for the instance variables instead of using them directly - # because the getters defined by property! ensures that the value is not a nil - options = DB::Pool::Options.new( - initial_pool_size: 0, - max_pool_size: max_capacity, - max_idle_pool_size: idle_capacity, - checkout_timeout: timeout - ) - - DB::Pool(HTTP::Client).new(options) do - next make_client(url, force_resolve: true) - end - end -end - -struct CompanionConnectionPool - property! max_capacity : Int32 - property! idle_capacity : Int32 - property! timeout : Float64 - property pool : DB::Pool(HTTP::Client) - - def initialize(*, @max_capacity : Int32 = 5, idle_capacity : Int32? = nil, @timeout : Float64 = 5.0) - if idle_capacity.nil? - @idle_capacity = @max_capacity - else - @idle_capacity = idle_capacity - end - - options = DB::Pool::Options.new( - initial_pool_size: 0, - max_pool_size: max_capacity, - max_idle_pool_size: idle_capacity.not_nil!, - checkout_timeout: timeout - ) - - @pool = DB::Pool(HTTP::Client).new(options) do - companion = CONFIG.invidious_companion.sample - next make_client(companion.private_url, use_http_proxy: false) - end - end - - def client(&) - conn = pool.checkout - - begin - response = yield conn - rescue ex - conn.close - - companion = CONFIG.invidious_companion.sample - conn = make_client(companion.private_url, use_http_proxy: false) - - response = yield conn - ensure - pool.release(conn) - end - - response - end -end +private YTIMG_POOLS = {} of String => Invidious::ConnectionPool::Pool def add_yt_headers(request) request.headers.delete("User-Agent") if request.headers["User-Agent"] == "Crystal" @@ -169,7 +64,7 @@ def get_ytimg_pool(subdomain) return pool else LOGGER.info("ytimg_pool: Creating a new HTTP pool for \"https://#{subdomain}.ytimg.com\"") - pool = YoutubeConnectionPool.new( + pool = Invidious::ConnectionPool::Pool.new( URI.parse("https://#{subdomain}.ytimg.com"), max_capacity: CONFIG.pool_size, idle_capacity: CONFIG.idle_pool_size From 27b66d288acca1ba27d3bc1927523ab05f809ead Mon Sep 17 00:00:00 2001 From: syeopite Date: Mon, 11 Nov 2024 16:01:07 -0800 Subject: [PATCH 03/25] Move client logic file to connection subfolder --- .../{yt_backend/connection_pool.cr => connection/client.cr} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/invidious/{yt_backend/connection_pool.cr => connection/client.cr} (100%) diff --git a/src/invidious/yt_backend/connection_pool.cr b/src/invidious/connection/client.cr similarity index 100% rename from src/invidious/yt_backend/connection_pool.cr rename to src/invidious/connection/client.cr From 347273dbc2174ddc11029c1b9920491986f81298 Mon Sep 17 00:00:00 2001 From: syeopite Date: Mon, 11 Nov 2024 16:04:30 -0800 Subject: [PATCH 04/25] Move ytimg pool logic to Invidious::ConnectionPool --- src/invidious/connection/client.cr | 23 ----------------------- src/invidious/connection/pool.cr | 28 +++++++++++++++++++++++++--- src/invidious/routes/images.cr | 8 ++++---- 3 files changed, 29 insertions(+), 30 deletions(-) diff --git a/src/invidious/connection/client.cr b/src/invidious/connection/client.cr index 512a26f1..ab3f8c50 100644 --- a/src/invidious/connection/client.cr +++ b/src/invidious/connection/client.cr @@ -1,7 +1,3 @@ -# Mapping of subdomain => Invidious::ConnectionPool::Pool -# This is needed as we may need to access arbitrary subdomains of ytimg -private YTIMG_POOLS = {} of String => Invidious::ConnectionPool::Pool - def add_yt_headers(request) request.headers.delete("User-Agent") if request.headers["User-Agent"] == "Crystal" request.headers["User-Agent"] ||= "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/128.0.0.0 Safari/537.36" @@ -55,22 +51,3 @@ def make_configured_http_proxy_client password: config_proxy.password, ) end - -# Fetches a HTTP pool for the specified subdomain of ytimg.com -# -# Creates a new one when the specified pool for the subdomain does not exist -def get_ytimg_pool(subdomain) - if pool = YTIMG_POOLS[subdomain]? - return pool - else - LOGGER.info("ytimg_pool: Creating a new HTTP pool for \"https://#{subdomain}.ytimg.com\"") - pool = Invidious::ConnectionPool::Pool.new( - URI.parse("https://#{subdomain}.ytimg.com"), - max_capacity: CONFIG.pool_size, - idle_capacity: CONFIG.idle_pool_size - ) - YTIMG_POOLS[subdomain] = pool - - return pool - end -end diff --git a/src/invidious/connection/pool.cr b/src/invidious/connection/pool.cr index ee69eb40..7aca09f9 100644 --- a/src/invidious/connection/pool.cr +++ b/src/invidious/connection/pool.cr @@ -13,7 +13,7 @@ module Invidious::ConnectionPool def initialize( @max_capacity : Int32 = 5, @idle_capacity : Int32? = nil, - @timeout : Float64 = 5.0 + @timeout : Float64 = 5.0, ) @pool = build_pool() end @@ -80,9 +80,8 @@ module Invidious::ConnectionPool *, @max_capacity : Int32 = 5, @idle_capacity : Int32? = nil, - @timeout : Float64 = 5.0 + @timeout : Float64 = 5.0, ) - @url = url @pool = build_pool() end @@ -113,4 +112,27 @@ module Invidious::ConnectionPool class Error < Exception end + + # Mapping of subdomain => Invidious::ConnectionPool::Pool + # This is needed as we may need to access arbitrary subdomains of ytimg + private YTIMG_POOLS = {} of String => Invidious::ConnectionPool::Pool + + # Fetches a HTTP pool for the specified subdomain of ytimg.com + # + # Creates a new one when the specified pool for the subdomain does not exist + def self.get_ytimg_pool(subdomain) + if pool = YTIMG_POOLS[subdomain]? + return pool + else + LOGGER.info("ytimg_pool: Creating a new HTTP pool for \"https://#{subdomain}.ytimg.com\"") + pool = Invidious::ConnectionPool::Pool.new( + URI.parse("https://#{subdomain}.ytimg.com"), + max_capacity: CONFIG.pool_size, + idle_capacity: CONFIG.idle_pool_size + ) + YTIMG_POOLS[subdomain] = pool + + return pool + end + end end diff --git a/src/invidious/routes/images.cr b/src/invidious/routes/images.cr index 51d85dfe..88720b38 100644 --- a/src/invidious/routes/images.cr +++ b/src/invidious/routes/images.cr @@ -42,7 +42,7 @@ module Invidious::Routes::Images end begin - get_ytimg_pool(authority).client &.get(url, headers) do |resp| + Invidious::ConnectionPool.get_ytimg_pool(authority).client &.get(url, headers) do |resp| env.response.headers["Connection"] = "close" return self.proxy_image(env, resp) end @@ -65,7 +65,7 @@ module Invidious::Routes::Images end begin - get_ytimg_pool("i9").client &.get(url, headers) do |resp| + Invidious::ConnectionPool.get_ytimg_pool("i9").client &.get(url, headers) do |resp| return self.proxy_image(env, resp) end rescue ex @@ -111,7 +111,7 @@ module Invidious::Routes::Images if name == "maxres.jpg" build_thumbnails(id).each do |thumb| thumbnail_resource_path = "/vi/#{id}/#{thumb[:url]}.jpg" - if get_ytimg_pool("i").client &.head(thumbnail_resource_path, headers).status_code == 200 + if Invidious::ConnectionPool.get_ytimg_pool("i").client &.head(thumbnail_resource_path, headers).status_code == 200 name = thumb[:url] + ".jpg" break end @@ -127,7 +127,7 @@ module Invidious::Routes::Images end begin - get_ytimg_pool("i").client &.get(url, headers) do |resp| + Invidious::ConnectionPool.get_ytimg_pool("i").client &.get(url, headers) do |resp| return self.proxy_image(env, resp) end rescue ex From ef07f786f2a816da97e06d69902872d2665fd698 Mon Sep 17 00:00:00 2001 From: syeopite Date: Mon, 11 Nov 2024 16:26:58 -0800 Subject: [PATCH 05/25] Add config to set connection pool checkout timeout --- config/config.example.yml | 13 +++++++++++-- src/invidious.cr | 10 ++++++++-- src/invidious/config.cr | 3 +++ src/invidious/connection/pool.cr | 3 ++- 4 files changed, 24 insertions(+), 5 deletions(-) diff --git a/config/config.example.yml b/config/config.example.yml index 781d0961..1d308edb 100644 --- a/config/config.example.yml +++ b/config/config.example.yml @@ -216,7 +216,7 @@ https_only: false ## -## Idle size of the HTTP pool used to connect to youtube. Each +## Max idle size of the HTTP pool used to connect to youtube. Each ## domain ('youtube.com', 'ytimg.com', ...) has its own pool. ## ## When unset this value has the same value as pool_size @@ -224,8 +224,17 @@ https_only: false ## Accepted values: a positive integer ## Default: (internally this means that it has the same value as pool_size) ## -#idle pool_size: 100 +#idle_pool_size: 100 +## +## Amount of seconds to wait for a client to be free from the pool +## before raising an error +## +## +## Accepted values: a positive integer +## Default: 5 +## +#pool_checkout_timeout: 5 ## diff --git a/src/invidious.cr b/src/invidious.cr index ac9fa4b1..435005ac 100644 --- a/src/invidious.cr +++ b/src/invidious.cr @@ -92,14 +92,20 @@ SOFTWARE = { "branch" => "#{CURRENT_BRANCH}", } -YT_POOL = Invidious::ConnectionPool::Pool.new(YT_URL, max_capacity: CONFIG.pool_size, idle_capacity: CONFIG.idle_pool_size) +YT_POOL = Invidious::ConnectionPool::Pool.new( + YT_URL, + max_capacity: CONFIG.pool_size, + idle_capacity: CONFIG.idle_pool_size, + timeout: CONFIG.pool_checkout_timeout +) # Image request pool GGPHT_POOL = Invidious::ConnectionPool::Pool.new( URI.parse("https://yt3.ggpht.com"), max_capacity: CONFIG.pool_size, - idle_capacity: CONFIG.idle_pool_size + idle_capacity: CONFIG.idle_pool_size, + timeout: CONFIG.pool_checkout_timeout ) COMPANION_POOL = Invidious::ConnectionPool::CompanionPool.new( diff --git a/src/invidious/config.cr b/src/invidious/config.cr index 00897371..2319cca0 100644 --- a/src/invidious/config.cr +++ b/src/invidious/config.cr @@ -163,6 +163,9 @@ class Config # Idle pool size for HTTP requests to youtube.com and ytimg.com (each domain has a separate pool) property idle_pool_size : Int32? = nil + # Amount of seconds to wait for a client to be free from the pool before rasing an error + property pool_checkout_timeout : Int32 = 5 + # HTTP Proxy configuration property http_proxy : HTTPProxyConfig? = nil diff --git a/src/invidious/connection/pool.cr b/src/invidious/connection/pool.cr index 7aca09f9..07c2861c 100644 --- a/src/invidious/connection/pool.cr +++ b/src/invidious/connection/pool.cr @@ -128,7 +128,8 @@ module Invidious::ConnectionPool pool = Invidious::ConnectionPool::Pool.new( URI.parse("https://#{subdomain}.ytimg.com"), max_capacity: CONFIG.pool_size, - idle_capacity: CONFIG.idle_pool_size + idle_capacity: CONFIG.idle_pool_size, + timeout: CONFIG.pool_checkout_timeout ) YTIMG_POOLS[subdomain] = pool From 739c35050f21bae031612e8b2cd7f6fde3624dc3 Mon Sep 17 00:00:00 2001 From: syeopite Date: Mon, 11 Nov 2024 16:28:24 -0800 Subject: [PATCH 06/25] Typo --- config/config.example.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/config.example.yml b/config/config.example.yml index 1d308edb..1fc7b893 100644 --- a/config/config.example.yml +++ b/config/config.example.yml @@ -224,7 +224,7 @@ https_only: false ## Accepted values: a positive integer ## Default: (internally this means that it has the same value as pool_size) ## -#idle_pool_size: 100 +#idle_pool_size: ## ## Amount of seconds to wait for a client to be free from the pool From 4fd6f797e8db1beab0bfc12e11f166087421f7e8 Mon Sep 17 00:00:00 2001 From: syeopite Date: Tue, 12 Nov 2024 08:57:55 -0800 Subject: [PATCH 07/25] Remove redundant pool.release pool.checkout(&block) already ensures that the checked out item will be released back into the pool --- src/invidious/connection/pool.cr | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/invidious/connection/pool.cr b/src/invidious/connection/pool.cr index 07c2861c..89734d2f 100644 --- a/src/invidious/connection/pool.cr +++ b/src/invidious/connection/pool.cr @@ -46,8 +46,6 @@ module Invidious::ConnectionPool # Prevent broken client from being checked back into the pool http_client.close raise ConnectionPool::Error.new(ex.message, cause: ex) - ensure - pool.release(http_client) end rescue ex : DB::PoolTimeout # Failed to checkout a client From 0d5887c2575f9c1d99f479781bfac6f876c6b622 Mon Sep 17 00:00:00 2001 From: syeopite Date: Tue, 12 Nov 2024 09:07:18 -0800 Subject: [PATCH 08/25] Delete broken clients from the pool explicitly --- src/invidious/connection/pool.cr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/invidious/connection/pool.cr b/src/invidious/connection/pool.cr index 89734d2f..9bb0f5e0 100644 --- a/src/invidious/connection/pool.cr +++ b/src/invidious/connection/pool.cr @@ -42,9 +42,9 @@ module Invidious::ConnectionPool response = yield http_client return response - rescue ex : DB::Error + rescue ex # Prevent broken client from being checked back into the pool - http_client.close + pool.delete(http_client) raise ConnectionPool::Error.new(ex.message, cause: ex) end rescue ex : DB::PoolTimeout From 4ce0f775fbf1153f7493e891ae3cc18d31479d5a Mon Sep 17 00:00:00 2001 From: syeopite Date: Tue, 12 Nov 2024 09:19:43 -0800 Subject: [PATCH 09/25] Improve documentation of idle pool size --- config/config.example.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/config/config.example.yml b/config/config.example.yml index 1fc7b893..fc9fc72e 100644 --- a/config/config.example.yml +++ b/config/config.example.yml @@ -219,6 +219,13 @@ https_only: false ## Max idle size of the HTTP pool used to connect to youtube. Each ## domain ('youtube.com', 'ytimg.com', ...) has its own pool. ## +## This means that when releasing a connection back into the pool, it will +## be closed if there are already more than idle_pool_size connections within +## the pool +## +## Do note that idle connections are kept around forever without any way of +## timing them out. +## ## When unset this value has the same value as pool_size ## ## Accepted values: a positive integer From e9b2db4d802d8714a37101851e4325c585c613cf Mon Sep 17 00:00:00 2001 From: syeopite Date: Thu, 14 Nov 2024 16:20:23 -0800 Subject: [PATCH 10/25] Connection pool: ensure response is fully read The streaming API of HTTP::Client has an internal buffer that will continue to persist onto the next request unless the response is fully read. This commit privatizes the #client method of Pool and instead expose various HTTP request methods that will call and yield the underlying request and response. This way, we can ensure that the resposne is fully read before the client is passed back into the pool for another request. --- src/invidious/channels/channels.cr | 2 +- src/invidious/connection/pool.cr | 29 +++++++++++++++++++++++-- src/invidious/mixes.cr | 2 +- src/invidious/routes/api/manifest.cr | 6 ++--- src/invidious/routes/api/v1/videos.cr | 6 ++--- src/invidious/routes/channels.cr | 2 +- src/invidious/routes/embed.cr | 2 +- src/invidious/routes/errors.cr | 6 ++--- src/invidious/routes/feeds.cr | 5 +++-- src/invidious/routes/images.cr | 12 +++++----- src/invidious/routes/playlists.cr | 2 +- src/invidious/search/processors.cr | 6 ++--- src/invidious/yt_backend/youtube_api.cr | 16 ++++++-------- 13 files changed, 60 insertions(+), 36 deletions(-) diff --git a/src/invidious/channels/channels.cr b/src/invidious/channels/channels.cr index 65982325..f71c4293 100644 --- a/src/invidious/channels/channels.cr +++ b/src/invidious/channels/channels.cr @@ -166,7 +166,7 @@ def fetch_channel(ucid, pull_all_videos : Bool) } LOGGER.trace("fetch_channel: #{ucid} : Downloading RSS feed") - rss = YT_POOL.client &.get("/feeds/videos.xml?channel_id=#{ucid}").body + rss = YT_POOL.get("/feeds/videos.xml?channel_id=#{ucid}").body LOGGER.trace("fetch_channel: #{ucid} : Parsing RSS feed") rss = XML.parse(rss) diff --git a/src/invidious/connection/pool.cr b/src/invidious/connection/pool.cr index 9bb0f5e0..139c7e77 100644 --- a/src/invidious/connection/pool.cr +++ b/src/invidious/connection/pool.cr @@ -33,8 +33,33 @@ module Invidious::ConnectionPool # Returns the underlying `DB::Pool` object abstract def pool : DB::Pool(PoolClient) - # Checks out a client from the pool - def client(&) + {% for method in %w[get post put patch delete head options] %} + def {{method.id}}(*args, **kwargs) + self.client do | client | + client.{{method.id}}(*args, **kwargs) do | response | + + result = yield response + return result + + ensure + response.body_io?.try &. skip_to_end + end + end + end + + def {{method.id}}(*args, **kwargs) + self.client do | client | + return response = client.{{method.id}}(*args, **kwargs) + ensure + if response + response.body_io?.try &. skip_to_end + end + end + end + {% end %} + + # Checks out a client in the pool + private def client(&) pool.checkout do |http_client| # Proxy needs to be reinstated every time we get a client from the pool http_client.proxy = make_configured_http_proxy_client() if CONFIG.http_proxy diff --git a/src/invidious/mixes.cr b/src/invidious/mixes.cr index 28ff0ff6..6728ff47 100644 --- a/src/invidious/mixes.cr +++ b/src/invidious/mixes.cr @@ -26,7 +26,7 @@ def fetch_mix(rdid, video_id, cookies = nil, locale = nil) end video_id = "CvFH_6DNRCY" if rdid.starts_with? "OLAK5uy_" - response = YT_POOL.client &.get("/watch?v=#{video_id}&list=#{rdid}&gl=US&hl=en", headers) + response = YT_POOL.get("/watch?v=#{video_id}&list=#{rdid}&gl=US&hl=en", headers) initial_data = extract_initial_data(response.body) if !initial_data["contents"]["twoColumnWatchNextResults"]["playlist"]? diff --git a/src/invidious/routes/api/manifest.cr b/src/invidious/routes/api/manifest.cr index c27caad7..bc1258f5 100644 --- a/src/invidious/routes/api/manifest.cr +++ b/src/invidious/routes/api/manifest.cr @@ -26,7 +26,7 @@ module Invidious::Routes::API::Manifest end if dashmpd = video.dash_manifest_url - response = YT_POOL.client &.get(URI.parse(dashmpd).request_target) + response = YT_POOL.get(URI.parse(dashmpd).request_target) if response.status_code != 200 haltf env, status_code: response.status_code @@ -167,7 +167,7 @@ module Invidious::Routes::API::Manifest # /api/manifest/hls_playlist/* def self.get_hls_playlist(env) - response = YT_POOL.client &.get(env.request.path) + response = YT_POOL.get(env.request.path) if response.status_code != 200 haltf env, status_code: response.status_code @@ -223,7 +223,7 @@ module Invidious::Routes::API::Manifest # /api/manifest/hls_variant/* def self.get_hls_variant(env) - response = YT_POOL.client &.get(env.request.path) + response = YT_POOL.get(env.request.path) if response.status_code != 200 haltf env, status_code: response.status_code diff --git a/src/invidious/routes/api/v1/videos.cr b/src/invidious/routes/api/v1/videos.cr index 6a3eb8ae..9d10b0e1 100644 --- a/src/invidious/routes/api/v1/videos.cr +++ b/src/invidious/routes/api/v1/videos.cr @@ -106,7 +106,7 @@ module Invidious::Routes::API::V1::Videos # Auto-generated captions often have cues that aren't aligned properly with the video, # as well as some other markup that makes it cumbersome, so we try to fix that here if caption.name.includes? "auto-generated" - caption_xml = YT_POOL.client &.get(url).body + caption_xml = YT_POOL.get(url).body settings_field = { "Kind" => "captions", @@ -147,7 +147,7 @@ module Invidious::Routes::API::V1::Videos query_params = uri.query_params query_params["fmt"] = "vtt" uri.query_params = query_params - webvtt = YT_POOL.client &.get(uri.request_target).body + webvtt = YT_POOL.get(uri.request_target).body if webvtt.starts_with?("[a-zA-Z0-9_-]{11})"/).try &.["video_id"] env.params.query.delete_all("channel") diff --git a/src/invidious/routes/errors.cr b/src/invidious/routes/errors.cr index 1e9ab44e..2f35f050 100644 --- a/src/invidious/routes/errors.cr +++ b/src/invidious/routes/errors.cr @@ -9,10 +9,10 @@ module Invidious::Routes::ErrorRoutes item = md["id"] # Check if item is branding URL e.g. https://youtube.com/gaming - response = YT_POOL.client &.get("/#{item}") + response = YT_POOL.get("/#{item}") if response.status_code == 301 - response = YT_POOL.client &.get(URI.parse(response.headers["Location"]).request_target) + response = YT_POOL.get(URI.parse(response.headers["Location"]).request_target) end if response.body.empty? @@ -40,7 +40,7 @@ module Invidious::Routes::ErrorRoutes end # Check if item is video ID - if item.match(/^[a-zA-Z0-9_-]{11}$/) && YT_POOL.client &.head("/watch?v=#{item}").status_code != 404 + if item.match(/^[a-zA-Z0-9_-]{11}$/) && YT_POOL.head("/watch?v=#{item}").status_code != 404 env.response.headers["Location"] = url haltf env, status_code: 302 end diff --git a/src/invidious/routes/feeds.cr b/src/invidious/routes/feeds.cr index 7f9a0edb..cd0fd2de 100644 --- a/src/invidious/routes/feeds.cr +++ b/src/invidious/routes/feeds.cr @@ -160,8 +160,9 @@ module Invidious::Routes::Feeds "default" => "http://www.w3.org/2005/Atom", } - response = YT_POOL.client &.get("/feeds/videos.xml?channel_id=#{ucid}") + response = YT_POOL.get("/feeds/videos.xml?channel_id=#{ucid}") return error_atom(404, NotFoundException.new("Channel does not exist.")) if response.status_code == 404 + rss = XML.parse(response.body) videos = rss.xpath_nodes("//default:feed/default:entry", namespaces).map do |entry| @@ -304,7 +305,7 @@ module Invidious::Routes::Feeds end end - response = YT_POOL.client &.get("/feeds/videos.xml?playlist_id=#{plid}") + response = YT_POOL.get("/feeds/videos.xml?playlist_id=#{plid}") return error_atom(404, NotFoundException.new("Playlist does not exist.")) if response.status_code == 404 document = XML.parse(response.body) diff --git a/src/invidious/routes/images.cr b/src/invidious/routes/images.cr index 88720b38..e79a7432 100644 --- a/src/invidious/routes/images.cr +++ b/src/invidious/routes/images.cr @@ -12,7 +12,7 @@ module Invidious::Routes::Images end begin - GGPHT_POOL.client &.get(url, headers) do |resp| + GGPHT_POOL.get(url, headers) do |resp| return self.proxy_image(env, resp) end rescue ex @@ -42,7 +42,7 @@ module Invidious::Routes::Images end begin - Invidious::ConnectionPool.get_ytimg_pool(authority).client &.get(url, headers) do |resp| + Invidious::ConnectionPool.get_ytimg_pool(authority).get(url, headers) do |resp| env.response.headers["Connection"] = "close" return self.proxy_image(env, resp) end @@ -65,7 +65,7 @@ module Invidious::Routes::Images end begin - Invidious::ConnectionPool.get_ytimg_pool("i9").client &.get(url, headers) do |resp| + Invidious::ConnectionPool.get_ytimg_pool("i9").get(url, headers) do |resp| return self.proxy_image(env, resp) end rescue ex @@ -81,7 +81,7 @@ module Invidious::Routes::Images end begin - YT_POOL.client &.get(env.request.resource, headers) do |response| + YT_POOL.get(env.request.resource, headers) do |response| env.response.status_code = response.status_code response.headers.each do |key, value| if !RESPONSE_HEADERS_BLACKLIST.includes?(key.downcase) @@ -111,7 +111,7 @@ module Invidious::Routes::Images if name == "maxres.jpg" build_thumbnails(id).each do |thumb| thumbnail_resource_path = "/vi/#{id}/#{thumb[:url]}.jpg" - if Invidious::ConnectionPool.get_ytimg_pool("i").client &.head(thumbnail_resource_path, headers).status_code == 200 + if Invidious::ConnectionPool.get_ytimg_pool("i").head(thumbnail_resource_path, headers).status_code == 200 name = thumb[:url] + ".jpg" break end @@ -127,7 +127,7 @@ module Invidious::Routes::Images end begin - Invidious::ConnectionPool.get_ytimg_pool("i").client &.get(url, headers) do |resp| + Invidious::ConnectionPool.get_ytimg_pool("i").get(url, headers) do |resp| return self.proxy_image(env, resp) end rescue ex diff --git a/src/invidious/routes/playlists.cr b/src/invidious/routes/playlists.cr index f2213da4..cb24648f 100644 --- a/src/invidious/routes/playlists.cr +++ b/src/invidious/routes/playlists.cr @@ -464,7 +464,7 @@ module Invidious::Routes::Playlists # Undocumented, creates anonymous playlist with specified 'video_ids', max 50 videos def self.watch_videos(env) - response = YT_POOL.client &.get(env.request.resource) + response = YT_POOL.get(env.request.resource) if url = response.headers["Location"]? url = URI.parse(url).request_target return env.redirect url diff --git a/src/invidious/search/processors.cr b/src/invidious/search/processors.cr index 25edb936..4c635ab2 100644 --- a/src/invidious/search/processors.cr +++ b/src/invidious/search/processors.cr @@ -16,11 +16,11 @@ module Invidious::Search # Search a youtube channel # TODO: clean code, and rely more on YoutubeAPI def channel(query : Query) : Array(SearchItem) - response = YT_POOL.client &.get("/channel/#{query.channel}") + response = YT_POOL.get("/channel/#{query.channel}") if response.status_code == 404 - response = YT_POOL.client &.get("/user/#{query.channel}") - response = YT_POOL.client &.get("/c/#{query.channel}") if response.status_code == 404 + response = YT_POOL.get("/user/#{query.channel}") + response = YT_POOL.get("/c/#{query.channel}") if response.status_code == 404 initial_data = extract_initial_data(response.body) ucid = initial_data.dig?("header", "c4TabbedHeaderRenderer", "channelId").try(&.as_s?) raise ChannelSearchException.new(query.channel) if !ucid diff --git a/src/invidious/yt_backend/youtube_api.cr b/src/invidious/yt_backend/youtube_api.cr index b40092a1..9a0c1cf8 100644 --- a/src/invidious/yt_backend/youtube_api.cr +++ b/src/invidious/yt_backend/youtube_api.cr @@ -639,15 +639,13 @@ module YoutubeAPI LOGGER.trace("YoutubeAPI: POST data: #{data}") # Send the POST request - body = YT_POOL.client() do |client| - client.post(url, headers: headers, body: data.to_json) do |response| - if response.status_code != 200 - raise InfoException.new("Error: non 200 status code. Youtube API returned \ - status code #{response.status_code}. See \ - https://docs.invidious.io/youtube-errors-explained/ for troubleshooting.") - end - self._decompress(response.body_io, response.headers["Content-Encoding"]?) + body = YT_POOL.post(url, headers: headers, body: data.to_json) do |response| + if response.status_code != 200 + raise InfoException.new("Error: non 200 status code. Youtube API returned \ + status code #{response.status_code}. See \ + https://docs.invidious.io/youtube-errors-explained/ for troubleshooting.") end + self._decompress(response.body_io, response.headers["Content-Encoding"]?) end # Convert result to Hash @@ -695,7 +693,7 @@ module YoutubeAPI # Send the POST request begin - response = COMPANION_POOL.client &.post(endpoint, headers: headers, body: data.to_json) + response = COMPANION_POOL.post(endpoint, headers: headers, body: data.to_json) body = response.body if (response.status_code != 200) raise Exception.new( From 35ddc67a1d65373e094f4290c11ee6576f97bbb3 Mon Sep 17 00:00:00 2001 From: syeopite Date: Thu, 14 Nov 2024 16:59:17 -0800 Subject: [PATCH 11/25] Release client only when it still exists @pool.release should not be called when the client has already been deleted from the pool. --- src/invidious/connection/pool.cr | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/invidious/connection/pool.cr b/src/invidious/connection/pool.cr index 139c7e77..786bd3c6 100644 --- a/src/invidious/connection/pool.cr +++ b/src/invidious/connection/pool.cr @@ -60,21 +60,32 @@ module Invidious::ConnectionPool # Checks out a client in the pool private def client(&) - pool.checkout do |http_client| - # Proxy needs to be reinstated every time we get a client from the pool - http_client.proxy = make_configured_http_proxy_client() if CONFIG.http_proxy + # If a client has been deleted from the pool + # we won't try to release it + client_exists_in_pool = true - response = yield http_client + http_client = pool.checkout - return response - rescue ex - # Prevent broken client from being checked back into the pool - pool.delete(http_client) - raise ConnectionPool::Error.new(ex.message, cause: ex) - end + # Proxy needs to be reinstated every time we get a client from the pool + http_client.proxy = make_configured_http_proxy_client() if CONFIG.http_proxy + + response = yield http_client rescue ex : DB::PoolTimeout # Failed to checkout a client raise ConnectionPool::Error.new(ex.message, cause: ex) + rescue ex + # An error occurred with the client itself. + # Delete the client from the pool and close the connection + if http_client + client_exists_in_pool = false + @pool.delete(http_client) + http_client.close + end + + # Raise exception for outer methods to handle + raise ConnectionPool::Error.new(ex.message, cause: ex) + ensure + pool.release(http_client) if http_client && client_exists_in_pool end # Builds a connection pool From 734c84c3f596e52f08071689a557c26cd24fe7ef Mon Sep 17 00:00:00 2001 From: syeopite Date: Thu, 14 Nov 2024 17:21:04 -0800 Subject: [PATCH 12/25] Pool: Refactor logic for request methods Make non-block request method internally call the block based request method. --- src/invidious/connection/pool.cr | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/invidious/connection/pool.cr b/src/invidious/connection/pool.cr index 786bd3c6..5dd169fd 100644 --- a/src/invidious/connection/pool.cr +++ b/src/invidious/connection/pool.cr @@ -34,7 +34,7 @@ module Invidious::ConnectionPool abstract def pool : DB::Pool(PoolClient) {% for method in %w[get post put patch delete head options] %} - def {{method.id}}(*args, **kwargs) + def {{method.id}}(*args, **kwargs, &) self.client do | client | client.{{method.id}}(*args, **kwargs) do | response | @@ -48,12 +48,10 @@ module Invidious::ConnectionPool end def {{method.id}}(*args, **kwargs) - self.client do | client | - return response = client.{{method.id}}(*args, **kwargs) + {{method.id}}(*args, **kwargs) do | response | + return response ensure - if response - response.body_io?.try &. skip_to_end - end + response.body_io?.try &. skip_to_end end end {% end %} From 4c44601b175e396737a79ad804de15895a9e236a Mon Sep 17 00:00:00 2001 From: syeopite Date: Thu, 14 Nov 2024 19:47:58 -0800 Subject: [PATCH 13/25] Pool: remove redundant properties --- src/invidious/config.cr | 2 +- src/invidious/connection/pool.cr | 81 ++++++++++++-------------------- 2 files changed, 31 insertions(+), 52 deletions(-) diff --git a/src/invidious/config.cr b/src/invidious/config.cr index 2319cca0..5304296f 100644 --- a/src/invidious/config.cr +++ b/src/invidious/config.cr @@ -164,7 +164,7 @@ class Config property idle_pool_size : Int32? = nil # Amount of seconds to wait for a client to be free from the pool before rasing an error - property pool_checkout_timeout : Int32 = 5 + property pool_checkout_timeout : Float64 = 5 # HTTP Proxy configuration property http_proxy : HTTPProxyConfig? = nil diff --git a/src/invidious/connection/pool.cr b/src/invidious/connection/pool.cr index 5dd169fd..e9b0ca18 100644 --- a/src/invidious/connection/pool.cr +++ b/src/invidious/connection/pool.cr @@ -3,31 +3,26 @@ module Invidious::ConnectionPool # # Uses `DB::Pool` for the pooling logic abstract struct BaseConnectionPool(PoolClient) - # Returns the max size of the connection pool - getter max_capacity : Int32 - - # Returns the configured checkout time out - getter timeout : Float64 - - # Creates a connection pool with the provided options + # Creates a connection pool with the provided options, and client factory block. def initialize( - @max_capacity : Int32 = 5, - @idle_capacity : Int32? = nil, - @timeout : Float64 = 5.0, + *, + max_capacity : Int32 = 5, + idle_capacity : Int32? = nil, + timeout : Float64 = 5.0, + &client_factory : -> PoolClient ) - @pool = build_pool() - end - - # Returns the idle capacity for the connection pool; if unset this is the same as `max_capacity`. - # - # This means that when idle capacity is unset the pool will keep all connections around forever, all the - # way until it reaches max capacity. - def idle_capacity : Int32 - if (idle = @idle_capacity).nil? - return @max_capacity + if idle_capacity.nil? + idle_capacity = max_capacity end - return idle + pool_options = DB::Pool::Options.new( + initial_pool_size: 0, + max_pool_size: max_capacity, + max_idle_pool_size: idle_capacity, + checkout_timeout: timeout + ) + + @pool = DB::Pool(PoolClient).new(pool_options, &client_factory) end # Returns the underlying `DB::Pool` object @@ -85,43 +80,22 @@ module Invidious::ConnectionPool ensure pool.release(http_client) if http_client && client_exists_in_pool end - - # Builds a connection pool - private abstract def build_pool : DB::Pool(PoolClient) - - # Creates a `DB::Pool::Options` used for constructing `DB::Pool` - private def pool_options : DB::Pool::Options - return DB::Pool::Options.new( - initial_pool_size: 0, - max_pool_size: max_capacity, - max_idle_pool_size: idle_capacity, - checkout_timeout: timeout - ) - end end # A basic connection pool where each client within is set to connect to a single resource struct Pool < BaseConnectionPool(HTTP::Client) - # The url each client within the pool will connect to - getter url : URI getter pool : DB::Pool(HTTP::Client) # Creates a pool of clients that connects to the given url, with the provided options. def initialize( url : URI, *, - @max_capacity : Int32 = 5, - @idle_capacity : Int32? = nil, - @timeout : Float64 = 5.0, + max_capacity : Int32 = 5, + idle_capacity : Int32? = nil, + timeout : Float64 = 5.0, ) - @url = url - @pool = build_pool() - end - - # :inherit: - private def build_pool : DB::Pool(HTTP::Client) - return DB::Pool(HTTP::Client).new(pool_options) do - make_client(url, force_resolve: true) + super(max_capacity: max_capacity, idle_capacity: idle_capacity, timeout: timeout) do + next make_client(url, force_resolve: true) end end end @@ -133,11 +107,16 @@ module Invidious::ConnectionPool struct CompanionPool < BaseConnectionPool(HTTP::Client) getter pool : DB::Pool(HTTP::Client) - # :inherit: - private def build_pool : DB::Pool(HTTP::Client) - return DB::Pool(HTTP::Client).new(pool_options) do + # Creates a pool of clients with the provided options. + def initialize( + *, + max_capacity : Int32 = 5, + idle_capacity : Int32? = nil, + timeout : Float64 = 5.0, + ) + super(max_capacity: max_capacity, idle_capacity: idle_capacity, timeout: timeout) do companion = CONFIG.invidious_companion.sample - make_client(companion.private_url, use_http_proxy: false) + next make_client(companion.private_url, use_http_proxy: false) end end end From 692df7d8ead6d4cd7e918247de4c520dfae8857a Mon Sep 17 00:00:00 2001 From: syeopite Date: Thu, 14 Nov 2024 19:52:48 -0800 Subject: [PATCH 14/25] Simplify namespace calls to ConnectionPool --- src/invidious/connection/pool.cr | 4 ++-- src/invidious/routes/images.cr | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/invidious/connection/pool.cr b/src/invidious/connection/pool.cr index e9b0ca18..02935253 100644 --- a/src/invidious/connection/pool.cr +++ b/src/invidious/connection/pool.cr @@ -126,7 +126,7 @@ module Invidious::ConnectionPool # Mapping of subdomain => Invidious::ConnectionPool::Pool # This is needed as we may need to access arbitrary subdomains of ytimg - private YTIMG_POOLS = {} of String => Invidious::ConnectionPool::Pool + private YTIMG_POOLS = {} of String => ConnectionPool::Pool # Fetches a HTTP pool for the specified subdomain of ytimg.com # @@ -136,7 +136,7 @@ module Invidious::ConnectionPool return pool else LOGGER.info("ytimg_pool: Creating a new HTTP pool for \"https://#{subdomain}.ytimg.com\"") - pool = Invidious::ConnectionPool::Pool.new( + pool = ConnectionPool::Pool.new( URI.parse("https://#{subdomain}.ytimg.com"), max_capacity: CONFIG.pool_size, idle_capacity: CONFIG.idle_pool_size, diff --git a/src/invidious/routes/images.cr b/src/invidious/routes/images.cr index e79a7432..7097ab79 100644 --- a/src/invidious/routes/images.cr +++ b/src/invidious/routes/images.cr @@ -42,7 +42,7 @@ module Invidious::Routes::Images end begin - Invidious::ConnectionPool.get_ytimg_pool(authority).get(url, headers) do |resp| + ConnectionPool.get_ytimg_pool(authority).get(url, headers) do |resp| env.response.headers["Connection"] = "close" return self.proxy_image(env, resp) end @@ -65,7 +65,7 @@ module Invidious::Routes::Images end begin - Invidious::ConnectionPool.get_ytimg_pool("i9").get(url, headers) do |resp| + ConnectionPool.get_ytimg_pool("i9").get(url, headers) do |resp| return self.proxy_image(env, resp) end rescue ex @@ -111,7 +111,7 @@ module Invidious::Routes::Images if name == "maxres.jpg" build_thumbnails(id).each do |thumb| thumbnail_resource_path = "/vi/#{id}/#{thumb[:url]}.jpg" - if Invidious::ConnectionPool.get_ytimg_pool("i").head(thumbnail_resource_path, headers).status_code == 200 + if ConnectionPool.get_ytimg_pool("i").head(thumbnail_resource_path, headers).status_code == 200 name = thumb[:url] + ".jpg" break end @@ -127,7 +127,7 @@ module Invidious::Routes::Images end begin - Invidious::ConnectionPool.get_ytimg_pool("i").get(url, headers) do |resp| + ConnectionPool.get_ytimg_pool("i").get(url, headers) do |resp| return self.proxy_image(env, resp) end rescue ex From cdb6eef0551b30c111f9b3f669e8537464b34fd0 Mon Sep 17 00:00:00 2001 From: syeopite Date: Thu, 14 Nov 2024 20:05:53 -0800 Subject: [PATCH 15/25] Update comment on reiniting proxy of pooled client --- src/invidious/connection/pool.cr | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/invidious/connection/pool.cr b/src/invidious/connection/pool.cr index 02935253..4107ceae 100644 --- a/src/invidious/connection/pool.cr +++ b/src/invidious/connection/pool.cr @@ -59,7 +59,12 @@ module Invidious::ConnectionPool http_client = pool.checkout - # Proxy needs to be reinstated every time we get a client from the pool + # When the HTTP::Client connection is closed, the automatic reconnection + # feature will create a new IO to connect to the server with + # + # This new TCP IO will be a direct connection to the server and will not go + # through the proxy. As such we'll need to reinitialize the proxy connection + http_client.proxy = make_configured_http_proxy_client() if CONFIG.http_proxy response = yield http_client From a3864e6691189dc2746eedbbf0029d1ff31bf054 Mon Sep 17 00:00:00 2001 From: syeopite Date: Wed, 9 Apr 2025 16:50:00 -0700 Subject: [PATCH 16/25] Use non-streaming api when not invoked with block Defaulting to the streaming api of `HTTP::Client` causes some issues since the streaming respone content needs to be accessed through #body_io rather than #body --- src/invidious/connection/pool.cr | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/invidious/connection/pool.cr b/src/invidious/connection/pool.cr index 4107ceae..34ce49e2 100644 --- a/src/invidious/connection/pool.cr +++ b/src/invidious/connection/pool.cr @@ -29,6 +29,8 @@ module Invidious::ConnectionPool abstract def pool : DB::Pool(PoolClient) {% for method in %w[get post put patch delete head options] %} + # Streaming API for {{method.id.upcase}} request. + # The response will have its body as an `IO` accessed via `HTTP::Client::Response#body_io`. def {{method.id}}(*args, **kwargs, &) self.client do | client | client.{{method.id}}(*args, **kwargs) do | response | @@ -42,11 +44,11 @@ module Invidious::ConnectionPool end end + # Executes a {{method.id.upcase}} request. + # The response will have its body as a `String`, accessed via `HTTP::Client::Response#body`. def {{method.id}}(*args, **kwargs) - {{method.id}}(*args, **kwargs) do | response | - return response - ensure - response.body_io?.try &. skip_to_end + self.client do | client | + return client.{{method.id}}(*args, **kwargs) end end {% end %} From ebefa5e914b7298605bba1b6c6f99b08348f047e Mon Sep 17 00:00:00 2001 From: syeopite Date: Wed, 9 Apr 2025 18:29:00 -0700 Subject: [PATCH 17/25] Add tests for connection pool --- .../networking/connection_pool_spec.cr | 95 +++++++++++++++++++ spec/load_config.cr | 15 +++ 2 files changed, 110 insertions(+) create mode 100644 spec/helpers/networking/connection_pool_spec.cr create mode 100644 spec/load_config.cr diff --git a/spec/helpers/networking/connection_pool_spec.cr b/spec/helpers/networking/connection_pool_spec.cr new file mode 100644 index 00000000..949558b9 --- /dev/null +++ b/spec/helpers/networking/connection_pool_spec.cr @@ -0,0 +1,95 @@ +# Due to the way that specs are handled this file cannot be run +# together with everything else without causing a compile time error +# +# TODO: Allow running different isolated spec through make +# +# For now run this with `crystal spec -p spec/helpers/networking/connection_pool_spec.cr -Drunning_by_self` +{% skip_file unless flag?(:running_by_self) %} + +# Based on https://github.com/jgaskins/http_client/blob/958cf56064c0d31264a117467022b90397eb65d7/spec/http_client_spec.cr +require "wait_group" +require "uri" +require "http" +require "http/server" +require "http_proxy" + +require "db" +require "pg" +require "spectator" + +require "../../load_config" +require "../../../src/invidious/helpers/crystal_class_overrides" +require "../../../src/invidious/connection/*" + +server = HTTP::Server.new do |context| + request = context.request + response = context.response + + case {request.method, request.path} + when {"GET", "/get"} + response << "get" + when {"POST", "/post"} + response.status = :created + response << "post" + when {"GET", "/sleep"} + duration = request.query_params["duration_sec"].to_i.seconds + sleep duration + end +end + +spawn server.listen 12345 + +Fiber.yield + +Spectator.describe Invidious::ConnectionPool do + describe "Pool" do + it "Can make a requests through standard HTTP methods" do + pool = Invidious::ConnectionPool::Pool.new(URI.parse("http://localhost:12345"), max_capacity: 100) + + expect(pool.get("/get").body).to eq("get") + expect(pool.post("/post").body).to eq("post") + end + + it "Can make streaming requests" do + pool = Invidious::ConnectionPool::Pool.new(URI.parse("http://localhost:12345"), max_capacity: 100) + + expect(pool.get("/get") { |r| r.body_io.gets_to_end }).to eq("get") + expect(pool.get("/post") { |r| r.body }).to eq("") + expect(pool.post("/post") { |r| r.body_io.gets_to_end }).to eq("post") + end + + # it "Can checkout a client" do + # end + + it "Allows concurrent requests" do + pool = Invidious::ConnectionPool::Pool.new(URI.parse("http://localhost:12345"), max_capacity: 100) + responses = [] of HTTP::Client::Response + + WaitGroup.wait do |wg| + 100.times do + wg.spawn { responses << pool.get("/get") } + end + end + + expect(responses.map(&.body)).to eq(["get"] * 100) + end + + it "Raises on checkout timeout" do + pool = Invidious::ConnectionPool::Pool.new(URI.parse("http://localhost:12345"), max_capacity: 2, timeout: 0.01) + + # Long running requests + 2.times do + spawn { pool.get("/sleep?duration_sec=2") } + end + + Fiber.yield + + expect { pool.get("/get") }.to raise_error(Invidious::ConnectionPool::Error) + end + + it "Raises when an error is encounter" do + pool = Invidious::ConnectionPool::Pool.new(URI.parse("http://localhost:12345"), max_capacity: 100, timeout: 0.01) + expect { pool.get("/get") { raise IO::Error.new } }.to raise_error(Invidious::ConnectionPool::Error) + end + end +end diff --git a/spec/load_config.cr b/spec/load_config.cr new file mode 100644 index 00000000..07a63718 --- /dev/null +++ b/spec/load_config.cr @@ -0,0 +1,15 @@ +require "yaml" +require "log" + +abstract class Kemal::BaseLogHandler +end + +require "../src/invidious/config" +require "../src/invidious/jobs/base_job" +require "../src/invidious/jobs.cr" +require "../src/invidious/user/preferences.cr" +require "../src/invidious/helpers/logger" +require "../src/invidious/helpers/utils" + +CONFIG = Config.from_yaml(File.open("config/config.example.yml")) +HMAC_KEY = CONFIG.hmac_key From f32c9548597e813eec3f0bef217197b5135337c2 Mon Sep 17 00:00:00 2001 From: syeopite Date: Wed, 9 Apr 2025 18:30:28 -0700 Subject: [PATCH 18/25] Pool: Make Pool#client method public --- .../networking/connection_pool_spec.cr | 20 +++++++++++++++++-- src/invidious/connection/pool.cr | 6 +++--- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/spec/helpers/networking/connection_pool_spec.cr b/spec/helpers/networking/connection_pool_spec.cr index 949558b9..bef41772 100644 --- a/spec/helpers/networking/connection_pool_spec.cr +++ b/spec/helpers/networking/connection_pool_spec.cr @@ -58,8 +58,24 @@ Spectator.describe Invidious::ConnectionPool do expect(pool.post("/post") { |r| r.body_io.gets_to_end }).to eq("post") end - # it "Can checkout a client" do - # end + it "Allows more than one clients to be checked out (if applicable)" do + pool = Invidious::ConnectionPool::Pool.new(URI.parse("http://localhost:12345"), max_capacity: 100) + + pool.checkout do | client | + expect(pool.post("/post").body).to eq("post") + end + end + + it "Can make multiple requests with the same client" do + pool = Invidious::ConnectionPool::Pool.new(URI.parse("http://localhost:12345"), max_capacity: 100) + + pool.checkout do | client | + expect(client.get("/get").body).to eq("get") + expect(client.post("/post").body).to eq("post") + expect(client.get("/get").body).to eq("get") + end + + end it "Allows concurrent requests" do pool = Invidious::ConnectionPool::Pool.new(URI.parse("http://localhost:12345"), max_capacity: 100) diff --git a/src/invidious/connection/pool.cr b/src/invidious/connection/pool.cr index 34ce49e2..f5d22f77 100644 --- a/src/invidious/connection/pool.cr +++ b/src/invidious/connection/pool.cr @@ -32,7 +32,7 @@ module Invidious::ConnectionPool # Streaming API for {{method.id.upcase}} request. # The response will have its body as an `IO` accessed via `HTTP::Client::Response#body_io`. def {{method.id}}(*args, **kwargs, &) - self.client do | client | + self.checkout do | client | client.{{method.id}}(*args, **kwargs) do | response | result = yield response @@ -47,14 +47,14 @@ module Invidious::ConnectionPool # Executes a {{method.id.upcase}} request. # The response will have its body as a `String`, accessed via `HTTP::Client::Response#body`. def {{method.id}}(*args, **kwargs) - self.client do | client | + self.checkout do | client | return client.{{method.id}}(*args, **kwargs) end end {% end %} # Checks out a client in the pool - private def client(&) + def checkout(&) # If a client has been deleted from the pool # we won't try to release it client_exists_in_pool = true From fbccb6a2210b6eaad23900a27a21c828df31063d Mon Sep 17 00:00:00 2001 From: syeopite Date: Wed, 9 Apr 2025 18:34:11 -0700 Subject: [PATCH 19/25] Pool: raise custom error when `DB::PoolTimeout` --- src/invidious/connection/pool.cr | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/invidious/connection/pool.cr b/src/invidious/connection/pool.cr index f5d22f77..777188e5 100644 --- a/src/invidious/connection/pool.cr +++ b/src/invidious/connection/pool.cr @@ -72,7 +72,7 @@ module Invidious::ConnectionPool response = yield http_client rescue ex : DB::PoolTimeout # Failed to checkout a client - raise ConnectionPool::Error.new(ex.message, cause: ex) + raise ConnectionPool::PoolCheckoutError.new(ex.message) rescue ex # An error occurred with the client itself. # Delete the client from the pool and close the connection @@ -131,6 +131,10 @@ module Invidious::ConnectionPool class Error < Exception end + # Raised when the pool failed to get a client in time + class PoolCheckoutError < Error + end + # Mapping of subdomain => Invidious::ConnectionPool::Pool # This is needed as we may need to access arbitrary subdomains of ytimg private YTIMG_POOLS = {} of String => ConnectionPool::Pool From 7901906092b007e92140a2265f214b484cd575a3 Mon Sep 17 00:00:00 2001 From: syeopite Date: Thu, 10 Apr 2025 00:31:41 -0700 Subject: [PATCH 20/25] Merge companion and standard pool into one --- .../networking/connection_pool_spec.cr | 23 +++---- src/invidious.cr | 19 ++++-- src/invidious/connection/pool.cr | 62 ++++--------------- 3 files changed, 37 insertions(+), 67 deletions(-) diff --git a/spec/helpers/networking/connection_pool_spec.cr b/spec/helpers/networking/connection_pool_spec.cr index bef41772..506cba6a 100644 --- a/spec/helpers/networking/connection_pool_spec.cr +++ b/spec/helpers/networking/connection_pool_spec.cr @@ -21,6 +21,8 @@ require "../../load_config" require "../../../src/invidious/helpers/crystal_class_overrides" require "../../../src/invidious/connection/*" +TEST_SERVER_URL = URI.parse("http://localhost:12345") + server = HTTP::Server.new do |context| request = context.request response = context.response @@ -44,14 +46,14 @@ Fiber.yield Spectator.describe Invidious::ConnectionPool do describe "Pool" do it "Can make a requests through standard HTTP methods" do - pool = Invidious::ConnectionPool::Pool.new(URI.parse("http://localhost:12345"), max_capacity: 100) + pool = Invidious::ConnectionPool::Pool.new(max_capacity: 100) { next make_client(TEST_SERVER_URL) } expect(pool.get("/get").body).to eq("get") expect(pool.post("/post").body).to eq("post") end it "Can make streaming requests" do - pool = Invidious::ConnectionPool::Pool.new(URI.parse("http://localhost:12345"), max_capacity: 100) + pool = Invidious::ConnectionPool::Pool.new(max_capacity: 100) { next make_client(TEST_SERVER_URL) } expect(pool.get("/get") { |r| r.body_io.gets_to_end }).to eq("get") expect(pool.get("/post") { |r| r.body }).to eq("") @@ -59,26 +61,25 @@ Spectator.describe Invidious::ConnectionPool do end it "Allows more than one clients to be checked out (if applicable)" do - pool = Invidious::ConnectionPool::Pool.new(URI.parse("http://localhost:12345"), max_capacity: 100) + pool = Invidious::ConnectionPool::Pool.new(max_capacity: 100) { next make_client(TEST_SERVER_URL) } - pool.checkout do | client | + pool.checkout do |client| expect(pool.post("/post").body).to eq("post") end end it "Can make multiple requests with the same client" do - pool = Invidious::ConnectionPool::Pool.new(URI.parse("http://localhost:12345"), max_capacity: 100) + pool = Invidious::ConnectionPool::Pool.new(max_capacity: 100) { next make_client(TEST_SERVER_URL) } - pool.checkout do | client | + pool.checkout do |client| expect(client.get("/get").body).to eq("get") expect(client.post("/post").body).to eq("post") expect(client.get("/get").body).to eq("get") end - end it "Allows concurrent requests" do - pool = Invidious::ConnectionPool::Pool.new(URI.parse("http://localhost:12345"), max_capacity: 100) + pool = Invidious::ConnectionPool::Pool.new(max_capacity: 100) { next make_client(TEST_SERVER_URL) } responses = [] of HTTP::Client::Response WaitGroup.wait do |wg| @@ -91,7 +92,7 @@ Spectator.describe Invidious::ConnectionPool do end it "Raises on checkout timeout" do - pool = Invidious::ConnectionPool::Pool.new(URI.parse("http://localhost:12345"), max_capacity: 2, timeout: 0.01) + pool = Invidious::ConnectionPool::Pool.new(max_capacity: 2, timeout: 0.01) { next make_client(TEST_SERVER_URL) } # Long running requests 2.times do @@ -103,8 +104,8 @@ Spectator.describe Invidious::ConnectionPool do expect { pool.get("/get") }.to raise_error(Invidious::ConnectionPool::Error) end - it "Raises when an error is encounter" do - pool = Invidious::ConnectionPool::Pool.new(URI.parse("http://localhost:12345"), max_capacity: 100, timeout: 0.01) + it "Raises when an error is encountered" do + pool = Invidious::ConnectionPool::Pool.new(max_capacity: 100) { next make_client(TEST_SERVER_URL) } expect { pool.get("/get") { raise IO::Error.new } }.to raise_error(Invidious::ConnectionPool::Error) end end diff --git a/src/invidious.cr b/src/invidious.cr index 435005ac..d773a6f5 100644 --- a/src/invidious.cr +++ b/src/invidious.cr @@ -93,25 +93,32 @@ SOFTWARE = { } YT_POOL = Invidious::ConnectionPool::Pool.new( - YT_URL, max_capacity: CONFIG.pool_size, idle_capacity: CONFIG.idle_pool_size, timeout: CONFIG.pool_checkout_timeout -) +) do + next make_client(YT_URL, force_resolve: true) +end # Image request pool +GGPHT_URL = URI.parse("https://yt3.ggpht.com") + GGPHT_POOL = Invidious::ConnectionPool::Pool.new( - URI.parse("https://yt3.ggpht.com"), max_capacity: CONFIG.pool_size, idle_capacity: CONFIG.idle_pool_size, timeout: CONFIG.pool_checkout_timeout -) +) do + next make_client(GGPHT_URL, force_resolve: true) +end -COMPANION_POOL = Invidious::ConnectionPool::CompanionPool.new( +COMPANION_POOL = Invidious::ConnectionPool::Pool.new( max_capacity: CONFIG.pool_size, idle_capacity: CONFIG.idle_pool_size -) +) do + companion = CONFIG.invidious_companion.sample + next make_client(companion.private_url, use_http_proxy: false) +end # CLI Kemal.config.extra_options do |parser| diff --git a/src/invidious/connection/pool.cr b/src/invidious/connection/pool.cr index 777188e5..2de18209 100644 --- a/src/invidious/connection/pool.cr +++ b/src/invidious/connection/pool.cr @@ -1,15 +1,15 @@ module Invidious::ConnectionPool - # The base connection pool that provides the underlying logic that all connection pools are based around - # - # Uses `DB::Pool` for the pooling logic - abstract struct BaseConnectionPool(PoolClient) + # A connection pool to reuse `HTTP::Client` connections + struct Pool + getter pool : DB::Pool(HTTP::Client) + # Creates a connection pool with the provided options, and client factory block. def initialize( *, max_capacity : Int32 = 5, idle_capacity : Int32? = nil, timeout : Float64 = 5.0, - &client_factory : -> PoolClient + &client_factory : -> HTTP::Client ) if idle_capacity.nil? idle_capacity = max_capacity @@ -22,12 +22,9 @@ module Invidious::ConnectionPool checkout_timeout: timeout ) - @pool = DB::Pool(PoolClient).new(pool_options, &client_factory) + @pool = DB::Pool(HTTP::Client).new(pool_options, &client_factory) end - # Returns the underlying `DB::Pool` object - abstract def pool : DB::Pool(PoolClient) - {% for method in %w[get post put patch delete head options] %} # Streaming API for {{method.id.upcase}} request. # The response will have its body as an `IO` accessed via `HTTP::Client::Response#body_io`. @@ -89,45 +86,6 @@ module Invidious::ConnectionPool end end - # A basic connection pool where each client within is set to connect to a single resource - struct Pool < BaseConnectionPool(HTTP::Client) - getter pool : DB::Pool(HTTP::Client) - - # Creates a pool of clients that connects to the given url, with the provided options. - def initialize( - url : URI, - *, - max_capacity : Int32 = 5, - idle_capacity : Int32? = nil, - timeout : Float64 = 5.0, - ) - super(max_capacity: max_capacity, idle_capacity: idle_capacity, timeout: timeout) do - next make_client(url, force_resolve: true) - end - end - end - - # A modified connection pool for the interacting with Invidious companion. - # - # The main difference is that clients in this pool are created with different urls - # based on what is randomly selected from the configured list of companions - struct CompanionPool < BaseConnectionPool(HTTP::Client) - getter pool : DB::Pool(HTTP::Client) - - # Creates a pool of clients with the provided options. - def initialize( - *, - max_capacity : Int32 = 5, - idle_capacity : Int32? = nil, - timeout : Float64 = 5.0, - ) - super(max_capacity: max_capacity, idle_capacity: idle_capacity, timeout: timeout) do - companion = CONFIG.invidious_companion.sample - next make_client(companion.private_url, use_http_proxy: false) - end - end - end - class Error < Exception end @@ -147,12 +105,16 @@ module Invidious::ConnectionPool return pool else LOGGER.info("ytimg_pool: Creating a new HTTP pool for \"https://#{subdomain}.ytimg.com\"") + url = URI.parse("https://#{subdomain}.ytimg.com") + pool = ConnectionPool::Pool.new( - URI.parse("https://#{subdomain}.ytimg.com"), max_capacity: CONFIG.pool_size, idle_capacity: CONFIG.idle_pool_size, timeout: CONFIG.pool_checkout_timeout - ) + ) do + next make_client(url, force_resolve: true) + end + YTIMG_POOLS[subdomain] = pool return pool From 8be9d85d51301c935f5bfc2367565e63b4eba460 Mon Sep 17 00:00:00 2001 From: syeopite Date: Thu, 10 Apr 2025 00:39:29 -0700 Subject: [PATCH 21/25] Fix ameba complaints --- .ameba.yml | 2 +- spec/helpers/networking/connection_pool_spec.cr | 10 +++++----- spec/{load_config.cr => load_config_helper.cr} | 0 3 files changed, 6 insertions(+), 6 deletions(-) rename spec/{load_config.cr => load_config_helper.cr} (100%) diff --git a/.ameba.yml b/.ameba.yml index 36d7c48f..e9e67b16 100644 --- a/.ameba.yml +++ b/.ameba.yml @@ -25,7 +25,7 @@ Lint/NotNil: Lint/SpecFilename: Excluded: - - spec/parsers_helper.cr + - spec/*_helper.cr # diff --git a/spec/helpers/networking/connection_pool_spec.cr b/spec/helpers/networking/connection_pool_spec.cr index 506cba6a..3209d4e3 100644 --- a/spec/helpers/networking/connection_pool_spec.cr +++ b/spec/helpers/networking/connection_pool_spec.cr @@ -17,7 +17,7 @@ require "db" require "pg" require "spectator" -require "../../load_config" +require "../../load_config_helper" require "../../../src/invidious/helpers/crystal_class_overrides" require "../../../src/invidious/connection/*" @@ -55,15 +55,15 @@ Spectator.describe Invidious::ConnectionPool do it "Can make streaming requests" do pool = Invidious::ConnectionPool::Pool.new(max_capacity: 100) { next make_client(TEST_SERVER_URL) } - expect(pool.get("/get") { |r| r.body_io.gets_to_end }).to eq("get") - expect(pool.get("/post") { |r| r.body }).to eq("") - expect(pool.post("/post") { |r| r.body_io.gets_to_end }).to eq("post") + expect(pool.get("/get", &.body_io.gets_to_end)).to eq("get") + expect(pool.get("/post", &.body)).to eq("") + expect(pool.post("/post", &.body_io.gets_to_end)).to eq("post") end it "Allows more than one clients to be checked out (if applicable)" do pool = Invidious::ConnectionPool::Pool.new(max_capacity: 100) { next make_client(TEST_SERVER_URL) } - pool.checkout do |client| + pool.checkout do |_| expect(pool.post("/post").body).to eq("post") end end diff --git a/spec/load_config.cr b/spec/load_config_helper.cr similarity index 100% rename from spec/load_config.cr rename to spec/load_config_helper.cr From 24a6c31b186841dd325a7f537b2192ad65dd9f51 Mon Sep 17 00:00:00 2001 From: syeopite Date: Thu, 10 Apr 2025 00:40:38 -0700 Subject: [PATCH 22/25] Remove extraneous space --- src/invidious/connection/pool.cr | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/invidious/connection/pool.cr b/src/invidious/connection/pool.cr index 2de18209..682e7cb9 100644 --- a/src/invidious/connection/pool.cr +++ b/src/invidious/connection/pool.cr @@ -31,12 +31,10 @@ module Invidious::ConnectionPool def {{method.id}}(*args, **kwargs, &) self.checkout do | client | client.{{method.id}}(*args, **kwargs) do | response | - result = yield response return result - ensure - response.body_io?.try &. skip_to_end + response.body_io?.try &.skip_to_end end end end @@ -85,7 +83,6 @@ module Invidious::ConnectionPool pool.release(http_client) if http_client && client_exists_in_pool end end - class Error < Exception end From 0865bc55fd78edc66fb128b90d2a381c6d4dc8ff Mon Sep 17 00:00:00 2001 From: syeopite Date: Thu, 10 Apr 2025 00:42:43 -0700 Subject: [PATCH 23/25] Remove idle_pool_size config Clients created when idle_capacity is reached but not max_capacity are discarded as soon as the client is checked back into the pool, not when the connection is closed. This means that allowing idle_capacity to be lower than max_capacity essentially just makes the remaining clients a checkout timeout deterrent that gets thrown away as soon as it is used. Not useful for reusing connections whatsoever during peak load times --- config/config.example.yml | 19 ------------------- src/invidious.cr | 3 --- src/invidious/config.cr | 2 -- src/invidious/connection/pool.cr | 8 +------- 4 files changed, 1 insertion(+), 31 deletions(-) diff --git a/config/config.example.yml b/config/config.example.yml index fc9fc72e..b849c116 100644 --- a/config/config.example.yml +++ b/config/config.example.yml @@ -214,25 +214,6 @@ https_only: false ## #pool_size: 100 - -## -## Max idle size of the HTTP pool used to connect to youtube. Each -## domain ('youtube.com', 'ytimg.com', ...) has its own pool. -## -## This means that when releasing a connection back into the pool, it will -## be closed if there are already more than idle_pool_size connections within -## the pool -## -## Do note that idle connections are kept around forever without any way of -## timing them out. -## -## When unset this value has the same value as pool_size -## -## Accepted values: a positive integer -## Default: (internally this means that it has the same value as pool_size) -## -#idle_pool_size: - ## ## Amount of seconds to wait for a client to be free from the pool ## before raising an error diff --git a/src/invidious.cr b/src/invidious.cr index d773a6f5..e39d08c4 100644 --- a/src/invidious.cr +++ b/src/invidious.cr @@ -94,7 +94,6 @@ SOFTWARE = { YT_POOL = Invidious::ConnectionPool::Pool.new( max_capacity: CONFIG.pool_size, - idle_capacity: CONFIG.idle_pool_size, timeout: CONFIG.pool_checkout_timeout ) do next make_client(YT_URL, force_resolve: true) @@ -106,7 +105,6 @@ GGPHT_URL = URI.parse("https://yt3.ggpht.com") GGPHT_POOL = Invidious::ConnectionPool::Pool.new( max_capacity: CONFIG.pool_size, - idle_capacity: CONFIG.idle_pool_size, timeout: CONFIG.pool_checkout_timeout ) do next make_client(GGPHT_URL, force_resolve: true) @@ -114,7 +112,6 @@ end COMPANION_POOL = Invidious::ConnectionPool::Pool.new( max_capacity: CONFIG.pool_size, - idle_capacity: CONFIG.idle_pool_size ) do companion = CONFIG.invidious_companion.sample next make_client(companion.private_url, use_http_proxy: false) diff --git a/src/invidious/config.cr b/src/invidious/config.cr index 5304296f..d3a96cff 100644 --- a/src/invidious/config.cr +++ b/src/invidious/config.cr @@ -160,8 +160,6 @@ class Config # Max pool size for HTTP requests to youtube.com and ytimg.com (each domain has a separate pool) property pool_size : Int32 = 100 - # Idle pool size for HTTP requests to youtube.com and ytimg.com (each domain has a separate pool) - property idle_pool_size : Int32? = nil # Amount of seconds to wait for a client to be free from the pool before rasing an error property pool_checkout_timeout : Float64 = 5 diff --git a/src/invidious/connection/pool.cr b/src/invidious/connection/pool.cr index 682e7cb9..b2b54f0e 100644 --- a/src/invidious/connection/pool.cr +++ b/src/invidious/connection/pool.cr @@ -7,18 +7,13 @@ module Invidious::ConnectionPool def initialize( *, max_capacity : Int32 = 5, - idle_capacity : Int32? = nil, timeout : Float64 = 5.0, &client_factory : -> HTTP::Client ) - if idle_capacity.nil? - idle_capacity = max_capacity - end - pool_options = DB::Pool::Options.new( initial_pool_size: 0, max_pool_size: max_capacity, - max_idle_pool_size: idle_capacity, + max_idle_pool_size: max_capacity, checkout_timeout: timeout ) @@ -106,7 +101,6 @@ module Invidious::ConnectionPool pool = ConnectionPool::Pool.new( max_capacity: CONFIG.pool_size, - idle_capacity: CONFIG.idle_pool_size, timeout: CONFIG.pool_checkout_timeout ) do next make_client(url, force_resolve: true) From edf41badc83481ad320d06489bd47ec41c0b060d Mon Sep 17 00:00:00 2001 From: syeopite Date: Thu, 10 Apr 2025 00:48:21 -0700 Subject: [PATCH 24/25] Formatting --- src/invidious/connection/pool.cr | 1 + 1 file changed, 1 insertion(+) diff --git a/src/invidious/connection/pool.cr b/src/invidious/connection/pool.cr index b2b54f0e..3c6d747e 100644 --- a/src/invidious/connection/pool.cr +++ b/src/invidious/connection/pool.cr @@ -78,6 +78,7 @@ module Invidious::ConnectionPool pool.release(http_client) if http_client && client_exists_in_pool end end + class Error < Exception end From ccbbc453617d841c5020f20071a2ea6ec470979a Mon Sep 17 00:00:00 2001 From: syeopite Date: Thu, 10 Apr 2025 01:05:15 -0700 Subject: [PATCH 25/25] Ensure http-proxy is not used for companion --- src/invidious.cr | 1 + src/invidious/connection/pool.cr | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/invidious.cr b/src/invidious.cr index e39d08c4..0ae785df 100644 --- a/src/invidious.cr +++ b/src/invidious.cr @@ -112,6 +112,7 @@ end COMPANION_POOL = Invidious::ConnectionPool::Pool.new( max_capacity: CONFIG.pool_size, + reinitialize_proxy: false ) do companion = CONFIG.invidious_companion.sample next make_client(companion.private_url, use_http_proxy: false) diff --git a/src/invidious/connection/pool.cr b/src/invidious/connection/pool.cr index 3c6d747e..a97b9983 100644 --- a/src/invidious/connection/pool.cr +++ b/src/invidious/connection/pool.cr @@ -8,6 +8,7 @@ module Invidious::ConnectionPool *, max_capacity : Int32 = 5, timeout : Float64 = 5.0, + @reinitialize_proxy : Bool = true, # Whether or not http-proxy should be reinitialized on checkout &client_factory : -> HTTP::Client ) pool_options = DB::Pool::Options.new( @@ -57,7 +58,7 @@ module Invidious::ConnectionPool # This new TCP IO will be a direct connection to the server and will not go # through the proxy. As such we'll need to reinitialize the proxy connection - http_client.proxy = make_configured_http_proxy_client() if CONFIG.http_proxy + http_client.proxy = make_configured_http_proxy_client() if @reinitialize_proxy && CONFIG.http_proxy response = yield http_client rescue ex : DB::PoolTimeout