[PATCH 1/4] BZ 802571 added eager loading and other minor efficiency fixes

Tzu-Mainn Chen tzumainn at redhat.com
Fri Mar 23 13:52:45 UTC 2012


---
 src/app/controllers/deployments_controller.rb |    4 +-
 src/app/controllers/pools_controller.rb       |   12 ++++----
 src/app/controllers/users_controller.rb       |    4 +-
 src/app/models/instance.rb                    |    9 +++++++
 src/app/models/pool.rb                        |   32 ++++++++++++++++++------
 src/app/views/deployments/_list.html.haml     |    2 +-
 src/app/views/pools/_deployments.html.haml    |    2 +-
 src/app/views/pools/_pretty_list.html.haml    |    8 +++---
 8 files changed, 49 insertions(+), 24 deletions(-)

diff --git a/src/app/controllers/deployments_controller.rb b/src/app/controllers/deployments_controller.rb
index 3eb7aa4..504a050 100644
--- a/src/app/controllers/deployments_controller.rb
+++ b/src/app/controllers/deployments_controller.rb
@@ -383,7 +383,7 @@ class DeploymentsController < ApplicationController
       { :name => t("providers.provider"), :sortable => false }
     ]
     @pools = Pool.list_for_user(current_user, Privilege::CREATE, Deployment)
-    @deployments = Deployment.includes(:owner).
+    @deployments = Deployment.includes(:owner, :pool, :instances).
                               apply_filters(:preset_filter_id => params[:deployments_preset_filter], :search_filter => params[:deployments_search]).
                               list_for_user(current_user, Privilege::VIEW).
                               where('deployments.pool_id' => @pools).
@@ -410,7 +410,7 @@ class DeploymentsController < ApplicationController
   end
 
   def init_new_deployment_attrs
-    @deployables = Deployable.list_for_user(current_user, Privilege::USE).select{|d| d.catalogs.collect {|c| c.pool}.include?(@pool)}
+    @deployables = Deployable.includes({:catalogs => :pool}).list_for_user(current_user, Privilege::USE).select{|d| d.catalogs.collect{|c| c.pool}.include?(@pool)}
     @pools = Pool.list_for_user(current_user, Privilege::CREATE, Deployment)
     @deployable = params[:deployable_id] ? Deployable.find(params[:deployable_id]) : nil
     @realms = FrontendRealm.all
diff --git a/src/app/controllers/pools_controller.rb b/src/app/controllers/pools_controller.rb
index cfefe34..abc3203 100644
--- a/src/app/controllers/pools_controller.rb
+++ b/src/app/controllers/pools_controller.rb
@@ -47,21 +47,21 @@ class PoolsController < ApplicationController
     if filter_view?
       case @details_tab[:id]
       when 'pools'
-        @pools = Pool.apply_filters(:preset_filter_id => params[:pools_preset_filter], :search_filter => params[:pools_search]).
+        @pools = Pool.includes(:deployments, :instances).apply_filters(:preset_filter_id => params[:pools_preset_filter], :search_filter => params[:pools_search]).
                       list_for_user(current_user, Privilege::VIEW).list(sort_column(Pool), sort_direction).
                       paginate(:page => params[:page], :per_page => PER_PAGE)
       when 'instances'
         params[:instances_preset_filter] = "" unless params[:instances_preset_filter]
-        @instances = Instance.apply_filters(:preset_filter_id => params[:instances_preset_filter], :search_filter => params[:instances_search]).
+        @instances = Instance.includes({:provider_account => :provider}).apply_filters(:preset_filter_id => params[:instances_preset_filter], :search_filter => params[:instances_search]).
                               list_for_user(current_user, Privilege::VIEW).list(sort_column(Instance), sort_direction).
                               paginate(:page => params[:page], :per_page => PER_PAGE)
       when 'deployments'
-        @deployments = Deployment.apply_filters(:preset_filter_id => params[:deployments_preset_filter], :search_filter => params[:deployments_search]).
+        @deployments = Deployment.includes(:pool, :instances).apply_filters(:preset_filter_id => params[:deployments_preset_filter], :search_filter => params[:deployments_search]).
                                   list_for_user(current_user, Privilege::VIEW).list(sort_column(Deployment), sort_direction).
                                   paginate(:page => params[:page], :per_page => PER_PAGE)
       end
     else
-      @pools = Pool.list_for_user(current_user, Privilege::VIEW).list(sort_column(Pool), sort_direction).
+      @pools = Pool.includes(:deployments, :instances, :quota, :catalogs).list_for_user(current_user, Privilege::VIEW).list(sort_column(Pool), sort_direction).
                     paginate(:page => params[:page], :per_page => PER_PAGE)
     end
 
@@ -109,7 +109,7 @@ class PoolsController < ApplicationController
 
     details_tab_name = params[:details_tab].blank? ? 'deployments' : params[:details_tab]
     @details_tab = @tabs.find {|t| t[:id] == details_tab_name} || @tabs.first[:name].downcase
-    @deployments = @pool.deployments.
+    @deployments = @pool.deployments.includes(:owner, :pool, :instances, :events).
                          apply_filters(:preset_filter_id => params[:deployments_preset_filter], :search_filter => params[:deployments_search]).
                          list_for_user(current_user, Privilege::VIEW).
                          paginate(:page => params[:page], :per_page => PER_PAGE) if @details_tab[:id] == 'deployments'
@@ -326,7 +326,7 @@ class PoolsController < ApplicationController
     instances = current_user.owned_instances
     failed_instances = instances.failed
     @statistics = {
-              :pools_in_use => @user_pools.select { |pool| pool.instances.pending.count > 0 || pool.instances.deployed.count > 0 }.count,
+              :pools_in_use => @user_pools.select { |pool| pool.instances.pending_or_deployed.count > 0 }.count,
               :deployments => current_user.deployments.count,
               :instances => instances.count,
               :instances_pending => instances.select {|instance| instance.state == Instance::STATE_NEW || instance.state == Instance::STATE_PENDING}.count,
diff --git a/src/app/controllers/users_controller.rb b/src/app/controllers/users_controller.rb
index 93918bc..010ebe1 100644
--- a/src/app/controllers/users_controller.rb
+++ b/src/app/controllers/users_controller.rb
@@ -157,7 +157,7 @@ class UsersController < ApplicationController
   protected
 
   def load_users
-    @users = User.apply_filters(:preset_filter_id => params[:users_preset_filter], :search_filter => params[:users_search])
+    @users = User.includes(:quota).apply_filters(:preset_filter_id => params[:users_preset_filter], :search_filter => params[:users_search])
     sort_order = params[:sort_by].nil? ? "login" : params[:sort_by]
     # TODO: (lmartinc) Optimize this sort! hell!
     if sort_order == "percentage_quota_used"
@@ -165,7 +165,7 @@ class UsersController < ApplicationController
     elsif sort_order == "quota"
       @users.sort! {|x,y| (x.quota.maximum_running_instances and y.quota.maximum_running_instances) ? x.quota.maximum_running_instances <=> y.quota.maximum_running_instances : (x ? 1 : -1) }
     else
-      @users = User.apply_filters(:preset_filter_id => params[:users_preset_filter], :search_filter => params[:users_search]).order(sort_order)
+      @users = User.includes(:quota).apply_filters(:preset_filter_id => params[:users_preset_filter], :search_filter => params[:users_search]).order(sort_order)
     end
   end
 
diff --git a/src/app/models/instance.rb b/src/app/models/instance.rb
index 855045a..60a0f09 100644
--- a/src/app/models/instance.rb
+++ b/src/app/models/instance.rb
@@ -120,6 +120,7 @@ class Instance < ActiveRecord::Base
   scope :deployed,  :conditions => { :state => [STATE_RUNNING, STATE_SHUTTING_DOWN] }
   # FIXME: "pending" is misleading as it doesn't just cover STATE_PENDING
   scope :pending,   :conditions => { :state => [STATE_NEW, STATE_PENDING] }
+  scope :pending_or_deployed,   :conditions => { :state => [STATE_NEW, STATE_PENDING, STATE_RUNNING, STATE_SHUTTING_DOWN] }
   # FIXME: "failed" is misleading too...
   scope :failed,    :conditions => { :state => FAILED_STATES }
   scope :stopped,   :conditions => {:state => STATE_STOPPED}
@@ -478,6 +479,14 @@ class Instance < ActiveRecord::Base
     [STATE_RUNNING, STATE_SHUTTING_DOWN].include?(state)
   end
 
+  def stopped?
+    [STATE_STOPPED].include?(state)
+  end
+
+  def pending?
+    [STATE_NEW, STATE_PENDING].include?(state)
+  end
+
   def uptime
     deployed? ? (Time.now - time_last_running) : 0
   end
diff --git a/src/app/models/pool.rb b/src/app/models/pool.rb
index 9d1af3a..57a6caf 100644
--- a/src/app/models/pool.rb
+++ b/src/app/models/pool.rb
@@ -78,6 +78,22 @@ class Pool < ActiveRecord::Base
     instances.all? {|i| i.destroyable? }
   end
 
+  def failed_instances
+    instances.select {|instance| instance.failed?}
+  end
+
+  def not_stopped_instances
+    instances.select {|instance| !instance.stopped?}
+  end
+
+  def deployed_instances
+    instances.select {|instance| instance.deployed?}
+  end
+
+  def pending_instances
+    instances.select {|instance| instance.pending?}
+  end
+
   # TODO: Implement Alerts and Updates
   def statistics(user = nil)
     # TODO - Need to set up cache invalidation before this is safe
@@ -85,17 +101,17 @@ class Pool < ActiveRecord::Base
     max = quota.maximum_running_instances
     total = quota.running_instances
     avail = max - total unless max.nil?
-    failed = (user.nil? ? instances.failed :
-              instances.failed.list_for_user(user, Privilege::VIEW))
+    failed = (user.nil? || failed_instances.empty? ? failed_instances :
+              failed_instances.list_for_user(user, Privilege::VIEW))
     statistics = {
-      :cloud_providers => instances.collect{|i| i.provider_account}.uniq.count,
-      :deployments => deployments.count,
-      :total_instances => instances.not_stopped.count,
-      :instances_deployed => instances.deployed.count,
-      :instances_pending => instances.pending.count,
+      :cloud_providers => instances.includes(:provider_account).collect{|i| i.provider_account}.uniq.count,
+      :deployments => deployments.size,
+      :total_instances => not_stopped_instances.count,
+      :instances_deployed => deployed_instances.count,
+      :instances_pending => pending_instances.count,
       :instances_failed => failed,
       :instances_failed_visible_count => failed.count,
-      :instances_failed_count => instances.failed.count,
+      :instances_failed_count => failed_instances.count,
       :used_quota => quota.running_instances,
       :quota_percent => number_to_percentage(quota.percentage_used,
                                              :precision => 0),
diff --git a/src/app/views/deployments/_list.html.haml b/src/app/views/deployments/_list.html.haml
index b123219..d593cc6 100644
--- a/src/app/views/deployments/_list.html.haml
+++ b/src/app/views/deployments/_list.html.haml
@@ -36,7 +36,7 @@
     %td= deployment.created_at
     %td= deployment.deployable_xml.name
     %td= deployment.status
-    %td{:class => 'center'}= deployment.instances.count
+    %td{:class => 'center'}= deployment.instances.size
     %td= link_to deployment.pool.name, pool_path(deployment.pool, :view => :filter)
     %td= owner_name(deployment)
     %td= link_to(deployment.provider.provider_type.name, deployment.provider) if deployment.provider
diff --git a/src/app/views/pools/_deployments.html.haml b/src/app/views/pools/_deployments.html.haml
index b4aed1d..d91bff5 100644
--- a/src/app/views/pools/_deployments.html.haml
+++ b/src/app/views/pools/_deployments.html.haml
@@ -11,7 +11,7 @@
             %ul
               %li.right
                 %dt.instances.count= t 'instances.instances.other'
-                %dd= deployment.instances.count || 0
+                %dd= deployment.instances.size || 0
 
               %li.left
                 %dt=t :uptime
diff --git a/src/app/views/pools/_pretty_list.html.haml b/src/app/views/pools/_pretty_list.html.haml
index 541f85a..47e7a8b 100644
--- a/src/app/views/pools/_pretty_list.html.haml
+++ b/src/app/views/pools/_pretty_list.html.haml
@@ -4,7 +4,7 @@
 
 .content#pools-list
   - @pools.each do |pool|
-    %div.pool.overview{:id => ['pool', pool.id], :class => ('collapsed' unless pool.deployments.count > 0)}
+    %div.pool.overview{:id => ['pool', pool.id], :class => ('collapsed' unless pool.deployments.size > 0)}
       %header
         %h3.name
           = link_to pool.name, pool
@@ -12,10 +12,10 @@
           %ul.groups
             %li
               %dt=t 'deployments.deployments'
-              %dd.count.deployment= pool.deployments.count
+              %dd.count.deployment= pool.deployments.size
             %li
               %dt=t 'instances.instances.other'
-              %dd.count.instance= pool.instances.count
+              %dd.count.instance= pool.instances.size
               %dd.count.instance.pending= pool.instances.select {|i| i.state == Instance::STATE_PENDING }.length
               %dd.count.instance.failure= pool.instances.select {|i| i.state == Instance::STATE_ERROR || i.state == Instance::STATE_CREATE_FAILED }.length
             %li
@@ -25,7 +25,7 @@
         %a.control{:href => '#'}
           %span Expand/Collapse
       %div.content
-        - deployments = pool.deployments.list_for_user(current_user, Privilege::VIEW).ascending_by_name.paginate(:page => params[:page], :per_page => PER_PAGE)
+        - deployments = pool.deployments.includes(:owner, :pool, :instances, :events).list_for_user(current_user, Privilege::VIEW).ascending_by_name.paginate(:page => params[:page], :per_page => PER_PAGE)
         = render :partial => 'deployments', :locals => {:pool => pool, :deployments => deployments}
       %ul.content.actions
         %li
-- 
1.7.6.5




More information about the aeolus-devel mailing list