You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltacloud.apache.org by mf...@redhat.com on 2012/12/11 16:06:02 UTC

[PATCH core] CIMI: More database access optimization

From: Michal Fojtik <mf...@redhat.com>

* Reduce queries for Provider using INNER JOIN
* Get rid of 'context' variable in database helpers
* Don't query database twice when entity is created

Signed-off-by: Michal fojtik <mf...@redhat.com>
---
 server/lib/cimi/helpers/database_helper.rb | 31 +++++++++++++++++++++++-------
 server/lib/cimi/models/base.rb             | 20 +++----------------
 server/lib/cimi/models/machine.rb          | 17 ++++++++--------
 server/lib/cimi/models/machine_template.rb | 17 +++++++++++-----
 server/lib/cimi/models/volume.rb           |  8 ++++----
 5 files changed, 52 insertions(+), 41 deletions(-)

diff --git a/server/lib/cimi/helpers/database_helper.rb b/server/lib/cimi/helpers/database_helper.rb
index 6573a1b..090f6d8 100644
--- a/server/lib/cimi/helpers/database_helper.rb
+++ b/server/lib/cimi/helpers/database_helper.rb
@@ -1,18 +1,15 @@
 module Deltacloud
   module Helpers
 
+    require_relative '../../deltacloud/helpers/driver_helper.rb'
+
     module Database
+      include Deltacloud::Helpers::Drivers
 
       def test_environment?
         Deltacloud.test_environment?
       end
 
-      def store_attributes_for(model, values={})
-        return if test_environment?
-        return if model.nil? or values.empty?
-        current_db.entities.first_or_create(:be_kind => model.to_entity, :be_id => model.id).update(values)
-      end
-
       def load_attributes_for(model)
         return {} if test_environment?
         entity = get_entity(model)
@@ -26,7 +23,7 @@ module Deltacloud
       end
 
       def get_entity(model)
-        current_db.entities.first(:be_kind => model.to_entity, :be_id => model.id)
+        Deltacloud::Database::Entity.first(:be_kind => model.to_entity, :be_id => model.id, 'provider.driver' => driver_symbol.to_s, 'provider.url' => current_provider)
       end
 
       def current_provider
@@ -36,10 +33,30 @@ module Deltacloud
       # This method allows to store things into database based on current driver
       # and provider.
       #
+
       def current_db
         Deltacloud::Database::Provider.first_or_create(:driver => driver_symbol.to_s, :url => current_provider)
       end
 
+      def store_attributes_for(model, attrs={})
+        return if test_environment? or model.nil? or attrs.empty?
+        entity = get_entity(model) || current_db.entities.new(:be_kind => model.to_entity, :be_id => model.id)
+
+        entity.description = extract_attribute_value('description', attrs) if attrs.has_key? 'description'
+        entity.name = extract_attribute_value('name', attrs) if attrs.has_key? 'name'
+        entity.ent_properties = extract_attribute_value('properties', attrs).to_json if attrs.has_key? 'properties'
+
+        entity.save! && entity
+      end
+
+      # In XML serialization the values stored in attrs are arrays, dues to
+      # XmlSimple. This method will help extract values from them
+      #
+      def extract_attribute_value(name, attrs={})
+        return unless attrs[name]
+        attrs[name].is_a?(Array) ? attrs[name].first : attrs[name]
+      end
+
     end
   end
 
diff --git a/server/lib/cimi/models/base.rb b/server/lib/cimi/models/base.rb
index 2988459..40b3454 100644
--- a/server/lib/cimi/models/base.rb
+++ b/server/lib/cimi/models/base.rb
@@ -269,7 +269,10 @@ class CIMI::Model::Resource
   end
 end
 
+require_relative '../helpers/database_helper'
+
 class CIMI::Model::Base < CIMI::Model::Resource
+  extend Deltacloud::Helpers::Database
   #
   # Common attributes for all resources
   #
@@ -286,21 +289,4 @@ class CIMI::Model::Base < CIMI::Model::Resource
     self.class.new(attrs)
   end
 
-  class << self
-    def store_attributes_for(context, entity, attrs={})
-      stored_attributes = {}
-      stored_attributes[:description] = extract_attribute_value('description', attrs) if attrs['description']
-      stored_attributes[:name] = extract_attribute_value('name', attrs) if attrs['name']
-      stored_attributes[:ent_properties] = extract_attribute_value('properties', attrs).to_json if attrs['properties']
-      context.store_attributes_for(entity, stored_attributes)
-    end
-
-    # In XML serialization the values stored in attrs are arrays, dues to
-    # XmlSimple. This method will help extract values from them
-    #
-    def extract_attribute_value(name, attrs={})
-      return unless attrs[name]
-      attrs[name].is_a?(Array) ? attrs[name].first : attrs[name]
-    end
-  end
 end
diff --git a/server/lib/cimi/models/machine.rb b/server/lib/cimi/models/machine.rb
index 4f28a92..53bdd37 100644
--- a/server/lib/cimi/models/machine.rb
+++ b/server/lib/cimi/models/machine.rb
@@ -49,7 +49,8 @@ class CIMI::Model::Machine < CIMI::Model::Base
     json = JSON.parse(body)
     machine_template = json['machineTemplate']
     if !machine_template['href'].nil?
-      template = context.current_db.machine_templates.first(:id => machine_template['href'].split('/').last)
+      template = current_db.machine_templates.first(:id => machine_template['href'].split('/').last)
+      raise 'Could not find the MachineTemplate' if template.nil?
       hardware_profile_id = template.machine_config.split('/').last
       image_id = template.machine_image.split('/').last
     else
@@ -68,14 +69,14 @@ class CIMI::Model::Machine < CIMI::Model::Base
 
     # Store attributes that are not supported by the backend cloud to local
     # database:
-    store_attributes_for(context, instance, json)
+    store_attributes_for(instance, json)
     from_instance(instance, context)
   end
 
   def self.create_from_xml(body, context)
     xml = XmlSimple.xml_in(body)
     if xml['machineTemplate'][0]['href']
-      template = context.current_db.machine_templates.first(:id => xml['machineTemplate'][0]['href'].split('/').last)
+      template = current_db.machine_templates.first(:id => xml['machineTemplate'][0]['href'].split('/').last)
       hardware_profile_id = template.machine_config.split('/').last
       image_id = template.machine_image.split('/').last
     else
@@ -94,8 +95,8 @@ class CIMI::Model::Machine < CIMI::Model::Base
 
     # Store attributes that are not supported by the backend cloud to local
     # database:
-    store_attributes_for(context, instance, xml)
-    from_instance(instance, context)
+    entity = store_attributes_for(instance, xml)
+    from_instance(instance, context, entity.to_hash)
   end
 
   def perform(action, context, &block)
@@ -111,7 +112,7 @@ class CIMI::Model::Machine < CIMI::Model::Base
   end
 
   def self.delete!(id, context)
-    context.delete_attributes_for Instance.new(:id => id)
+    delete_attributes_for Instance.new(:id => id)
     context.driver.destroy_instance(context.credentials, id)
   end
 
@@ -139,10 +140,10 @@ class CIMI::Model::Machine < CIMI::Model::Base
   end
 
   private
-  def self.from_instance(instance, context)
+  def self.from_instance(instance, context, stored_attributes=nil)
     cpu =  memory = (instance.instance_profile.id == "opaque")? "n/a" : nil
     machine_conf = CIMI::Model::MachineConfiguration.find(instance.instance_profile.name, context)
-    stored_attributes = context.load_attributes_for(instance)
+    stored_attributes ||= load_attributes_for(instance)
     if stored_attributes[:property]
       stored_attributes[:property].merge!(convert_instance_properties(instance, context))
     else
diff --git a/server/lib/cimi/models/machine_template.rb b/server/lib/cimi/models/machine_template.rb
index 64fa26c..9d6dac4 100644
--- a/server/lib/cimi/models/machine_template.rb
+++ b/server/lib/cimi/models/machine_template.rb
@@ -44,9 +44,16 @@ class CIMI::Model::MachineTemplate < CIMI::Model::Base
   class << self
     def find(id, context)
       if id == :all
-        context.current_db.machine_templates.all.map { |t| from_db(t, context) }
+        Deltacloud::Database::MachineTemplate.all(
+          'provider.driver' => driver_symbol.to_s,
+          'provider.url' => current_provider
+        ).map { |t| from_db(t, context) }
       else
-        template = context.current_db.machine_templates.first(:id => id)
+        template = Deltacloud::Database::MachineTemplate.first(
+          'provider.driver' => driver_symbol.to_s,
+          'provider.url' => current_provider,
+          :id => id
+        )
         raise CIMI::Model::NotFound unless template
         from_db(template, context)
       end
@@ -54,7 +61,7 @@ class CIMI::Model::MachineTemplate < CIMI::Model::Base
 
     def create_from_json(body, context)
       json = JSON.parse(body)
-      new_template = context.current_db.machine_templates.new(
+      new_template = current_db.machine_templates.new(
         :name => json['name'],
         :description => json['description'],
         :machine_config => json['machineConfig']['href'],
@@ -69,7 +76,7 @@ class CIMI::Model::MachineTemplate < CIMI::Model::Base
 
     def create_from_xml(body, context)
       xml = XmlSimple.xml_in(body)
-      new_template = context.current_db.machine_templates.new(
+      new_template = current_db.machine_templates.new(
         :name => xml['name'].first,
         :description => xml['description'].first,
         :machine_config => xml['machineConfig'].first['href'],
@@ -83,7 +90,7 @@ class CIMI::Model::MachineTemplate < CIMI::Model::Base
     end
 
     def delete!(id, context)
-      context.current_db.machine_templates.first(:id => id).destroy
+      current_db.machine_templates.first(:id => id).destroy
     end
 
     private
diff --git a/server/lib/cimi/models/volume.rb b/server/lib/cimi/models/volume.rb
index a2904ce..e3b4d4a 100644
--- a/server/lib/cimi/models/volume.rb
+++ b/server/lib/cimi/models/volume.rb
@@ -101,12 +101,12 @@ class CIMI::Model::Volume < CIMI::Model::Base
       opts = {:capacity=>context.from_kibibyte(params[:capacity], "GB"), :snapshot_id=>params[:volume_image_id]}
     end
     storage_volume = context.driver.create_storage_volume(context.credentials, opts)
-    store_attributes_for(context, storage_volume, data)
-    from_storage_volume(storage_volume, context)
+    entity = store_attributes_for(storage_volume, data)
+    from_storage_volume(storage_volume, context, entity.to_hash)
   end
 
-  def self.from_storage_volume(volume, context)
-    stored_attributes = context.load_attributes_for(volume)
+  def self.from_storage_volume(volume, context, stored_attributes=nil)
+    stored_attributes ||= load_attributes_for(volume)
     self.new( { :name => stored_attributes[:name] || volume.id,
                 :description => stored_attributes[:description] || 'Description of Volume',
                 :property => stored_attributes[:property],
-- 
1.8.0.1