-
-
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 1 commit
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 |
|---|---|---|
|
|
@@ -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
Member
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
Member
Author
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
Member
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.
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.