Fixes #33974 - Change the pool size to threads + 4 #1161

ShimShtein · 2024-04-24T11:31:12Z

According to an investigation described in https://community.theforeman.org/t/rails-connection-pool-size-optimizations/36675 foreman process spawns 4 additional threads that consume DB connection during the startup. Hence the amount of acctive DB connections should be the amount of puma threads + 4 additional threads.

ShimShtein · 2024-04-24T13:04:19Z

@pablomh Thanks for all the testing and experimenting.
What do you think, is it enough?

pablomh · 2024-04-24T20:58:57Z

Hi @ShimShtein, according to our data, this change should be very welcome :)

These are the results of our testing that justify this change:

Chart showing the number of registration errors with different DB pool values when performing concurrent registrations on a "medium" sized Satellite server

In it we can see where only modifying the DB pool value improves the registration success rate, getting no errors until reaching >200 concurrent registration requests with values of DB pool >= 8.
Chart showing the number of DB pool exhaustion errors (could not obtain a connection from the pool within 5.000 seconds (waited 5.000 seconds); all pooled connections were in use) with different DB pool values when performing concurrent registrations on a "medium" sized Satellite server

In it we can see how all the DB pool exhaustion errors dissapear with values of DB pool >= 8.
Chart showing the number of DB pool connection retries appearing in the logs when performing concurrent registrations on a "medium" sized Satellite server

In it we can see how all the DB pool connection retries dissapear with values of DB pool >= 9.

Based on this testing, we agree that the minimum Foreman DB pool value where all these issues are addressed is 9.

ehelms

@evgeni / @ekohl any concerns with this approach to changing the pool size?

ehelms · 2024-05-03T12:11:49Z

@ShimShtein This will reset the default value, to really fix this we need to also reset this in the installer through a migration.

ShimShtein · 2024-05-05T10:59:13Z

@ehelms Added a migration PR: theforeman/foreman-installer#937

ekohl · 2024-05-08T15:36:50Z

manifests/init.pp

@@ -226,7 +226,7 @@
  String[1] $db_password = $foreman::params::db_password,
  Optional[String[1]] $db_sslmode = undef,
  Optional[String[1]] $db_root_cert = undef,
-  Integer[0] $db_pool = 5,
+  Integer[0] $db_pool = 9,


Should we make this:

Suggested change

Integer[0] $db_pool = 9,

Optional[Integer[0]] $db_pool = undef,

That way, if we change it in the future it automatically gets recalculated. We would need to change the part in config.pp to deal with it by using pick() instead of max().

The DB migration could then simply be .delete('db_pool') if db_pool == 5.

ShimShtein · 2024-05-09T08:46:03Z

@ekohl fixed both here and the installer

ekohl · 2024-05-10T10:40:14Z

manifests/config.pp

@@ -68,7 +68,9 @@
    'database'  => $foreman::db_database,
    'username'  => $foreman::db_username,
    'password'  => $foreman::db_password,
-    'db_pool'   => max($foreman::db_pool, $foreman::foreman_service_puma_threads_max),
+    # Set the pool size to at least the amount of puma threads + 4 threads that are spawned automatically by the process.


It would be nice to document that the 4 is just needed for Katello.

ekohl · 2024-05-10T10:41:53Z

manifests/init.pp

@@ -226,7 +226,7 @@
  String[1] $db_password = $foreman::params::db_password,
  Optional[String[1]] $db_sslmode = undef,
  Optional[String[1]] $db_root_cert = undef,
-  Integer[0] $db_pool = 5,
+  Optional[Integer[0]] $db_pool = undef,


On line 87 there's documentation for this parameter that's shown in the installer when using --full-help. Perhaps you can update it to describe the behavior.

ekohl · 2024-05-10T10:44:06Z

manifests/config.pp

-    'db_pool'   => max($foreman::db_pool, $foreman::foreman_service_puma_threads_max),
+    # Set the pool size to at least the amount of puma threads + 4 threads that are spawned automatically by the process.
+    # db_pool is optional, and undef means "use default" and the second part of the max statement will be set
+    'db_pool'   => max(pick($foreman::db_pool, 0), $foreman::foreman_service_puma_threads_max + 4),


I'd keep this simple so you still provide the user with full control. For example if the + 4 part does give problems you still provide a way to override it.

Suggested change

'db_pool' => max(pick($foreman::db_pool, 0), $foreman::foreman_service_puma_threads_max + 4),

'db_pool' => pick($foreman::db_pool, $foreman::foreman_service_puma_threads_max + 4),

ekohl

@evgeni Debian 12 is failing:

   current directory:
  /usr/share/foreman/vendor/ruby/3.1.0/gems/psych-5.1.2/ext/psych
  /usr/bin/ruby3.1 -I /usr/lib/ruby/vendor_ruby -r
  ./siteconf20240512-4799-xgqy0g.rb extconf.rb
  checking for yaml.h... no
  yaml.h not found
  *** extconf.rb failed ***
  Could not create Makefile due to some reason, probably lack of necessary
  libraries and/or headers.  Check the mkmf.log file for more details.  You may
  need configuration options.

But I thought theforeman/foreman-packaging@60435ac would address that.

ekohl · 2024-05-13T20:08:55Z

manifests/init.pp

-# $db_pool::                      Database 'production' size of connection pool. When running as a reverse proxy,
-#                                 the value of `$foreman_service_puma_threads_max` is used if it's higher than `$db_pool`.
+# $db_pool::                      Database 'production' size of connection pool. If the value is not set, it will be
+#                                 set by default to the amount of puma threads + 4 (for threads that are spawn by the system)


Took me a while to figure this out. Would this be clearer?

Suggested change

# set by default to the amount of puma threads + 4 (for threads that are spawn by the system)

# set by default to the amount of puma threads + 4 (for internal system threads)

According to an investigation described in https://community.theforeman.org/t/rails-connection-pool-size-optimizations/36675 foreman process spawns 4 additional threads that consume DB connection during the startup. Hence the amount of acctive DB connections should be the amount of puma threads + 4 additional threads.

ShimShtein · 2024-05-16T09:36:31Z

Should be a bit more clear now.

pr-processor bot added Not yet reviewed Waiting on contributor labels Apr 24, 2024

ShimShtein force-pushed the update_db_pool_size branch from 7bda3a9 to 0780c61 Compare April 24, 2024 11:33

pr-processor bot removed the Waiting on contributor label Apr 24, 2024

ShimShtein changed the title ~~Fixes #1976728 - Change the pool size to threads + 4~~ Fixes #33974 - Change the pool size to threads + 4 Apr 24, 2024

ehelms approved these changes Apr 29, 2024

View reviewed changes

pr-processor bot removed the Not yet reviewed label Apr 29, 2024

evgeni approved these changes May 3, 2024

View reviewed changes

ekohl reviewed May 8, 2024

View reviewed changes

ShimShtein force-pushed the update_db_pool_size branch from 0780c61 to 6d1f3b7 Compare May 9, 2024 08:42

ekohl reviewed May 10, 2024

View reviewed changes

ShimShtein force-pushed the update_db_pool_size branch from 6d1f3b7 to 5b43a41 Compare May 12, 2024 13:25

ekohl reviewed May 13, 2024

View reviewed changes

ShimShtein force-pushed the update_db_pool_size branch from 5b43a41 to c20c0ca Compare May 16, 2024 09:34

ShimShtein force-pushed the update_db_pool_size branch from c20c0ca to 19f67cb Compare May 16, 2024 09:36

ekohl approved these changes May 16, 2024

View reviewed changes

ekohl merged commit b2feca1 into theforeman:master May 16, 2024
15 of 17 checks passed

ekohl added the Backwards incompatible label May 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #33974 - Change the pool size to threads + 4 #1161

Fixes #33974 - Change the pool size to threads + 4 #1161

ShimShtein commented Apr 24, 2024

ShimShtein commented Apr 24, 2024

pablomh commented Apr 24, 2024

ehelms left a comment

ehelms commented May 3, 2024

ShimShtein commented May 5, 2024

ekohl May 8, 2024

ShimShtein commented May 9, 2024

ekohl May 10, 2024

ekohl May 10, 2024

ekohl May 10, 2024

ekohl left a comment

ekohl May 13, 2024

ShimShtein commented May 16, 2024

	Integer[0] $db_pool = 9,
	Optional[Integer[0]] $db_pool = undef,

	'db_pool' => max(pick($foreman::db_pool, 0), $foreman::foreman_service_puma_threads_max + 4),
	'db_pool' => pick($foreman::db_pool, $foreman::foreman_service_puma_threads_max + 4),

	# set by default to the amount of puma threads + 4 (for threads that are spawn by the system)
	# set by default to the amount of puma threads + 4 (for internal system threads)

Fixes #33974 - Change the pool size to threads + 4 #1161

Fixes #33974 - Change the pool size to threads + 4 #1161

Conversation

ShimShtein commented Apr 24, 2024

ShimShtein commented Apr 24, 2024

pablomh commented Apr 24, 2024

ehelms left a comment

Choose a reason for hiding this comment

ehelms commented May 3, 2024

ShimShtein commented May 5, 2024

ekohl May 8, 2024

Choose a reason for hiding this comment

ShimShtein commented May 9, 2024

ekohl May 10, 2024

Choose a reason for hiding this comment

ekohl May 10, 2024

Choose a reason for hiding this comment

ekohl May 10, 2024

Choose a reason for hiding this comment

ekohl left a comment

Choose a reason for hiding this comment

ekohl May 13, 2024

Choose a reason for hiding this comment

ShimShtein commented May 16, 2024