From 8f5301d20d73a84cc10ea2591414e45cf3d1fa79 Mon Sep 17 00:00:00 2001
From: Sai Sindhur Malleni <smalleni@redhat.com>
Date: Fri, 10 Jan 2020 14:28:32 -0500
Subject: [PATCH] Fix performance regression due to reduced number of keystone
 workers

The puppet-keystone project consolidated keystone applications:

  https://github.com/openstack/puppet-keystone/commit/ace7aeb3b71b39a59f92fbc9e7f676a70c9a797a#diff-e5968f40345cf5d3be0539fbba87f787

This effectively reduces the number of available workers to process
keystone requests since only half the processes are deployed now that
the applications are consolidated [0]. As a result, we see a performance
regression [1] when interacting with keystone.

For example, in earlier versions, an 8 core machine would spawn four
processes to serve the keystone-main application. Another four processes
would be spawned to serve the keystone-admin application. Now, we only
have 4 processes total to serve the entire keystone application. Due to
the difference in the number of workers deployed by default, users will
experience reduced performance out-of-the-box.

This patch not only brings performance back to the previous levels, it
is also safe to implement as we are not changing the number of workers
when compared to previous releases and only bringing parity.

[0] https://github.com/openstack/puppet-openstacklib/blob/master/lib/facter/os_workers.rb#L33
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1789495

Co-Authored-By: Takashi Kajinami <tkajinam@redhat.com>
Depends-on: https://review.opendev.org/#/c/705041/
Change-Id: Icf9acaa106af705fa249a2ef2abca9f9a91fba59
---
 manifests/wsgi/apache.pp                                | 4 ++--
 releasenotes/notes/double-workers-b9e340a18a5e9823.yaml | 5 +++++
 spec/classes/keystone_db_postgresql_spec.rb             | 4 ++--
 spec/classes/keystone_wsgi_apache_spec.rb               | 8 ++++----
 4 files changed, 13 insertions(+), 8 deletions(-)
 create mode 100644 releasenotes/notes/double-workers-b9e340a18a5e9823.yaml

diff --git a/manifests/wsgi/apache.pp b/manifests/wsgi/apache.pp
index ddb17e9b7..17bd889b9 100644
--- a/manifests/wsgi/apache.pp
+++ b/manifests/wsgi/apache.pp
@@ -32,7 +32,7 @@
 #
 # [*workers*]
 #   (Optional) Number of WSGI workers to spawn.
-#   Defaults to $::os_workers
+#   Defaults to $::os_workers_keystone
 #
 # [*ssl_cert*]
 #   (Optional) Path to SSL certificate
@@ -139,7 +139,7 @@ class keystone::wsgi::apache (
   $api_port                          = 5000,
   $path                              = '/',
   $ssl                               = true,
-  $workers                           = $::os_workers,
+  $workers                           = $::os_workers_keystone,
   $ssl_cert                          = undef,
   $ssl_key                           = undef,
   $ssl_chain                         = undef,
diff --git a/releasenotes/notes/double-workers-b9e340a18a5e9823.yaml b/releasenotes/notes/double-workers-b9e340a18a5e9823.yaml
new file mode 100644
index 000000000..86615856a
--- /dev/null
+++ b/releasenotes/notes/double-workers-b9e340a18a5e9823.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Workers are raised to 2 x os_workers, so that we have as many workers as
+    the one we had before we merged 2 keystone services(public and admin).
diff --git a/spec/classes/keystone_db_postgresql_spec.rb b/spec/classes/keystone_db_postgresql_spec.rb
index 2f6b7e39f..94158f817 100644
--- a/spec/classes/keystone_db_postgresql_spec.rb
+++ b/spec/classes/keystone_db_postgresql_spec.rb
@@ -30,8 +30,8 @@ describe 'keystone::db::postgresql' do
     context "on #{os}" do
       let (:facts) do
         facts.merge(OSDefaults.get_facts({
-          :os_workers     => 8,
-          :concat_basedir => '/var/lib/puppet/concat'
+          :os_workers_keystone => 8,
+          :concat_basedir      => '/var/lib/puppet/concat'
         }))
       end
 
diff --git a/spec/classes/keystone_wsgi_apache_spec.rb b/spec/classes/keystone_wsgi_apache_spec.rb
index 1c5339678..6c30e3fe0 100644
--- a/spec/classes/keystone_wsgi_apache_spec.rb
+++ b/spec/classes/keystone_wsgi_apache_spec.rb
@@ -22,7 +22,7 @@ describe 'keystone::wsgi::apache' do
         :bind_host                   => nil,
         :bind_port                   => 5000,
         :group                       => 'keystone',
-        :workers                     => facts[:os_workers],
+        :workers                     => facts[:os_workers_keystone],
         :threads                     => 1,
         :user                        => 'keystone',
         :priority                    => '10',
@@ -193,9 +193,9 @@ describe 'keystone::wsgi::apache' do
     context "on #{os}" do
       let (:facts) do
         facts.merge!(OSDefaults.get_facts({
-          :os_workers     => 8,
-          :concat_basedir => '/var/lib/puppet/concat',
-          :fqdn           => 'some.host.tld',
+          :os_workers_keystone => 8,
+          :concat_basedir      => '/var/lib/puppet/concat',
+          :fqdn                => 'some.host.tld',
         }))
       end