Memoize root tenant

[lpichler]

First part for the BZ. (in other I will eliminate useless calls to reporting_available_fields)
From the BZ:

it takes 10-20 sec to add column to new report when report is based on big fields set like Virtual Machines

it is (also) because there is method reporting_available_fields called and there is thru other methods(MiqExpression.value2human) called Tenant.root_tenant which is producing 1 query for each call and for getting the just name of root tenant and name is unchangeable.

Numbers for MiqExpression.reporting_available_fields('Vm', nil):

before:
:total_queries: 249 (:rows_by_class Tenant:242 )

after:
:total_queries: 8 (:rows_by_class Tenant:1 )

Links

https://bugzilla.redhat.com/show_bug.cgi?id=1446542

@miq-bot assign @gtanzillo
@miq-bot add_label core, perfomance

[miq-bot]

@lpichler Cannot apply the following labels because they are not recognized: , perfomance

[isimluk]

I am thinking, what about we memoize root_tenant, i.e.:

    def self.root_tenant
      @root_tenant ||= in_my_region.roots.first
    end

I think the root_tenant shouldn't change, but its name could. Additionally, you wouldn't need this method (and modify callers).

Perhaps I am missing something?

[isimluk]

Ah, I see specs fail horribly after memoizing root_tenant.

What about if we do the same magic as the MiqServer.myserver does:

diff --git a/app/models/tenant.rb b/app/models/tenant.rb
index 5c53243..7d1ef32 100644
--- a/app/models/tenant.rb
+++ b/app/models/tenant.rb
@@ -216,13 +216,11 @@ class Tenant < ApplicationRecord
   # from this tenant, all tenants are positioned
   #
   # @return [Tenant] the root tenant
-  def self.root_tenant
-    in_my_region.roots.first
-  end
+  cache_with_timeout(:root_tenant) { in_my_region.roots.first }
 
   # NOTE: returns the root tenant
   def self.seed
-    root_tenant || create!(:use_config_for_attributes => true) do |_|
+    in_my_region.roots.first || create!(:use_config_for_attributes => true) do |_|
       _log.info("Creating root tenant")
     end
   end
diff --git a/spec/support/evm_spec_helper.rb b/spec/support/evm_spec_helper.rb
index 2eafc0c..9ccb6a3 100644
--- a/spec/support/evm_spec_helper.rb
+++ b/spec/support/evm_spec_helper.rb
@@ -77,6 +77,7 @@ module EvmSpecHelper
   end
 
   def self.remote_miq_server(attrs = {})
+    Tenant.root_tenant_clear_cache
     Tenant.root_tenant || Tenant.create!(:use_config_for_attributes => false)
 
     FactoryGirl.create(:miq_server, attrs)

[lpichler]

@isimluk thanks a lot for looking on it, I tried it but it was causing failures in other specs
but after some playing, I realized that during switching rspec contexts the database is cleared out but not classes variables on the models and it was causing most of CI failures

• so I added it to clear_caches method after each test here 74d6415
• and I also found the problem with validation method, described in the commit
  ae66629

[lpichler]

@miq-bot remove_label wip

[lpichler]

@miq-bot add_label performance

[gtanzillo]

LGTM 👍 , Just need to fix typo in the method name.

[gtanzillo]

Typo, should be root_tenant_without_cache

[lpichler]

done 👍

[miq-bot]

Checked commits lpichler/manageiq@74d6415~...0ea303b with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

[kbrock]

👍 Surprised this would save us that much. LGTM

[lpichler]

@miq-bot add_label fine/yes

[simaishi]

Fine backport details:

$ git log -1
commit 728f98b46df442dbd68563db37050cddd5c6f3d6
Author: Gregg Tanzillo <[email protected]>
Date:   Thu Jun 1 06:38:28 2017 -0400

    Merge pull request #15191 from lpichler/memoize_root_tenant
    
    Memoize root tenant
    (cherry picked from commit 8a0fa8e92224feb4fbb62933a7a9a5ce2534f5dd)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1461958