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

ShimShtein commented 4 months ago

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 commented 4 months ago

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

pablomh commented 4 months ago

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 commented 4 months ago

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

ShimShtein commented 4 months ago

ShimShtein commented 4 months ago

@ekohl fixed both here and the installer

ShimShtein commented 4 months ago

Should be a bit more clear now.

theforeman / puppet-foreman