-
-
Notifications
You must be signed in to change notification settings - Fork 2k
Expose database connection pool settings #5333
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,93 @@ db: | |
| port: 5432 | ||
| dbname: invidious | ||
|
|
||
|
|
||
| ## ----------------------------------- | ||
| ## Database connection pool settings | ||
| ## ----------------------------------- | ||
|
|
||
| ## | ||
| ## The maximum allowed number of connections within the connection pool. | ||
| ## When this number is reached and no connections are free, Invidious will | ||
| ## wait for up to `checkout_timeout` before raising an exception. | ||
| ## | ||
| ## Note: Setting to 0 will disable this limit and allow the the pool to | ||
| ## grow indefinitely. | ||
| ## | ||
| ## Accepted values: a positive integer | ||
| ## Default: 100 | ||
| ## | ||
| #max_pool_size: 100 | ||
|
|
||
| ## | ||
| ## How many idle connections should be allowed to exist within the connection pool. | ||
| ## | ||
| ## There is no concept of whether a connection is closed or open, or how long it has been | ||
| ## idly sitting by. Instead Idle connections are defined as any connections that are not | ||
| ## currently being used. When this number is exceeded but the amount of connections is still | ||
| ## under max_pool_size, new connections will be created on a checkout but immediately closed | ||
| ## and destroyed during an release operation. | ||
| ## | ||
| ## For the most part, this should just be set to the same number as the `max_pool_size`. | ||
| ## | ||
| ## Accepted values: a positive integer | ||
| ## Default: 100 | ||
| ## | ||
| #max_idle_pool_size: 100 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I originally also exposed the idle pool size config for the HTTP connection pool in #5081 but later reverted the change after I learnt what idle actually meant in the context of the connection pool. I've added it here for completeness but I don't know if there's actually any benefits in exposing this setting over making it just always equal the max pool size. |
||
|
|
||
| ## | ||
| ## The amount of connections to establish on start-up. | ||
| ## | ||
| ## When Invidious starts up, the database connection pool is immediately populated | ||
| ## with this many connections. This could allow an instance to avoid potentially experiencing | ||
| ## any degraded service during start-up; since a good amount of connections will be available | ||
| ## from the start rather than needing to be created on the fly as requests come in. | ||
| ## | ||
| ## | ||
| ## Accepted values: a positive integer | ||
| ## Default: 1 | ||
| ## | ||
| #initial_pool_size : 1 | ||
|
|
||
| ## | ||
| ## How long to wait (in seconds) before timing out during a checkout | ||
| ## | ||
| ## Accepted values: a positive float | ||
| ## Default: 5.0 | ||
| ## | ||
| #checkout_timeout: 5.0 | ||
|
|
||
| ## | ||
| ## How many attempts to retry a connection | ||
| ## | ||
| ## This allows Invidious to gracefully handle network problems that can | ||
| ## cause a connection to fail or be unable to get established in the first | ||
| ## place. | ||
| ## | ||
| ## Note: The mechanisms of the underlying library will first try to reuse all the | ||
| ## currently available idle connections within the pool, as well as one additional attempt, | ||
| ## at no delay. Afterwards, it will began counting down the `retry_attempts` and wait | ||
| ## `retry_delay` seconds between each attempt. | ||
| ## | ||
| ## This heuristic ensures that unnecessary new connections are not being made for no reason, and | ||
| ## in the event of widespread network failures, will help to throw out dead connections from the | ||
| ## pool. | ||
| ## | ||
| ## Accepted values: a positive integer | ||
| ## Default: 5 | ||
| ## | ||
| #retry_attempts: 5 | ||
|
|
||
| ## | ||
| ## How long to wait (in seconds) between each retry attempt | ||
| ## | ||
| ## Note: See description for `retry_attempts` for more information | ||
| ## | ||
| ## Accepted values: a positive float | ||
| ## Default: 1.0 | ||
| ## | ||
| #retry_delay: 1.0 | ||
|
|
||
| ## | ||
| ## Database configuration using a single URI. This is an | ||
| ## alternative to the 'db' parameter above. If both forms | ||
|
|
@@ -26,6 +113,10 @@ db: | |
| ## and append the 'host' parameter. E.g: | ||
| ## postgres://kemal:kemal@/invidious?host=/var/run/postgresql | ||
| ## | ||
| ## You can also configure the connection pool here by | ||
| ## adding query parameters with the same name as the value you | ||
| ## want to change. | ||
| ## | ||
| ## Accepted values: a postgres:// URI | ||
| ## Default: postgres://kemal:kemal@localhost:5432/invidious | ||
| ## | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,52 @@ struct DBConfig | |
| property host : String | ||
| property port : Int32 | ||
| property dbname : String | ||
|
|
||
| # How many connections to construct on start-up, and keep it there. | ||
| property initial_pool_size : Int32 = 1 | ||
| # The maximum size of the connection pool | ||
| property max_pool_size : Int32 = 100 | ||
| # The maximum amount of idle connections within the pool | ||
| # idle connections are defined as created connections that are | ||
| # sitting around in the pool. Exceeding this number will cause new connections | ||
| # to be created on checkout and then simply dropped on release, till the maximum pool size | ||
| # from which there will be a checkout timeout. | ||
| property max_idle_pool_size : Int32 = 100 | ||
|
Comment on lines
+10
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should note two things. The value of
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure about forcing a user to change their default postgres configuration just to accommodate Invidious' default behavior. Maybe we could just reduce the default There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That will depend of the traffic of the instance. I think 32 or even 16 is fine. My instance used to run using a unix socket without pgbouncer (but with ~4 replicas, so at the end, the total connections to the database were 24 (4 replicas * 6 connections)) at all and everything worked fine. I tested using When using When using 2 threads and 50 connections, I got 1826req/s with What matters is the latency of each operation. I'm not a database expert, but I still think this setting will depend of the current setup of Invidious people have. People that use a shared database used along other services like me may need to set a higher People that use the Docker guide of https://docs.invidious.io/installation/#docker-compose-method-production should be fine since People that use Docker replicas or some sort of replicas, will have to adjust the |
||
| # The maximum amount of seconds to wait for a connection to become | ||
| # available when all connections can be checked out, and the pool has | ||
| # reached its maximum size. | ||
| property checkout_timeout : Float32 = 5.0 | ||
| # The number of tries allowed to establish a connection, reconnect, or retry | ||
| # the command in case of any network errors. | ||
| property retry_attempts : Int32 = 5 | ||
| # The number of seconds between each retry | ||
| property retry_delay : Float32 = 1.0 | ||
|
|
||
| def to_url | ||
| URI.new( | ||
| scheme: "postgres", | ||
| user: user, | ||
| password: password, | ||
| host: host, | ||
| port: port, | ||
| path: dbname, | ||
| query: get_connection_pool_query_string | ||
| ) | ||
| end | ||
|
|
||
| # Creates the query parameters for configuring the connection pool | ||
| private def get_connection_pool_query_string | ||
| {% begin %} | ||
| {% pool_vars = @type.instance_vars.reject { |v| {"user", "password", "host", "port", "dbname"}.includes?(v.name.stringify) } %} | ||
| {% raise "Error unable to isolate database connection pool properties" if pool_vars.size > 6 %} | ||
|
|
||
| URI::Params.build do | build | | ||
| {% for vars in pool_vars %} | ||
| build.add {{vars.name.stringify}}, {{vars.name}}.to_s | ||
| {% end %} | ||
| end | ||
| {% end %} | ||
| end | ||
| end | ||
|
|
||
| struct SocketBindingConfig | ||
|
|
@@ -287,18 +333,23 @@ class Config | |
| # Build database_url from db.* if it's not set directly | ||
| if config.database_url.to_s.empty? | ||
| if db = config.db | ||
| config.database_url = URI.new( | ||
| scheme: "postgres", | ||
| user: db.user, | ||
| password: db.password, | ||
| host: db.host, | ||
| port: db.port, | ||
| path: db.dbname, | ||
| ) | ||
| config.database_url = db.to_url | ||
| else | ||
| puts "Config: Either database_url or db.* is required" | ||
| exit(1) | ||
| end | ||
| else | ||
| # Add default connection pool settings as needed | ||
| db_url_query_params = config.database_url.query_params | ||
|
|
||
| {% begin %} | ||
| {% pool_vars = DBConfig.instance_vars.reject { |v| {"user", "password", "host", "port", "dbname"}.includes?(v.name.stringify) } %} | ||
| {% for vars in pool_vars %} | ||
| db_url_query_params[{{vars.name.stringify}}] = db_url_query_params[{{vars.name.stringify}}]? || {{vars.default_value}}.to_s | ||
| {% end %} | ||
| {% end %} | ||
|
|
||
| config.database_url.query_params = db_url_query_params | ||
| end | ||
|
|
||
| # Check if the socket configuration is valid | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a probably a non issue but
Although all of this is nested under the db field, the options here are still really similar to the ones for the HTTP connection pool which might get confusing. And this could be made worse if we ever decide to offer more granular control for the latter (eg configuring the pool options for each domain) which will lead to a lot of essentially duplicate options.