Omer Frenkel has uploaded a new change for review.

Change subject: core: fix deadlock on update vm version
......................................................................

core: fix deadlock on update vm version

When a new template version created,
all vms using 'latest' of the template are updated to the new version.
seems that the update of template child count inside the insertVmStatic
stored procedure caused deadlock in db, because it would do select
count(*) on the table, without taking any lock.

in this patch the insert/delete vm static stored procedures are changed
to take a lock on the template, and the update line changed to +1 / -1
instead of count(*), because of the lock, now it is safe to do.

Change-Id: I2dfdb556e66658c3ef630cf71fba5f902ea26862
Signed-off-by: Omer Frenkel <ofren...@redhat.com>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java
M packaging/dbscripts/vms_sp.sql
2 files changed, 11 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/61/28861/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java
index 62afdf1..c91652b 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java
@@ -58,7 +58,6 @@
 import org.ovirt.engine.core.common.utils.Pair;
 import org.ovirt.engine.core.common.validation.group.CreateEntity;
 import org.ovirt.engine.core.compat.Guid;
-import org.ovirt.engine.core.compat.TransactionScopeOption;
 import org.ovirt.engine.core.dal.dbbroker.DbFacade;
 import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector;
 import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogableBase;
@@ -628,14 +627,14 @@
 
         // in case of new version of a template, update vms marked to use 
latest
         if (isTemplateVersion()) {
-            String jobId = 
SchedulerUtilQuartzImpl.getInstance().scheduleAOneTimeJob(this, 
"onTimerHandleVdsRecovering", new Class[0],
+            String jobId = 
SchedulerUtilQuartzImpl.getInstance().scheduleAOneTimeJob(this, 
"updateVmVersion", new Class[0],
                     new Object[0], 0, TimeUnit.SECONDS);
             updateVmsJobIdMap.put(getParameters().getBaseTemplateId(), jobId);
         }
     }
 
-    @OnTimerMethodAnnotation("onTimerHandleVdsRecovering")
-    public void onTimerHandleVdsRecovering() {
+    @OnTimerMethodAnnotation("updateVmVersion")
+    public void updateVmVersion() {
         for (Guid vmId : 
getVmDAO().getVmIdsForVersionUpdate(getParameters().getBaseTemplateId())) {
             // if the job was removed, stop executing, we probably have new 
version creation going on
             if 
(!updateVmsJobIdMap.containsKey(getParameters().getBaseTemplateId())) {
@@ -643,8 +642,6 @@
             }
             UpdateVmVersionParameters params = new 
UpdateVmVersionParameters(vmId);
             params.setSessionId(getParameters().getSessionId());
-            // execute in new transaction, as failure here should not fail 
template creation
-            
params.setTransactionScopeOption(TransactionScopeOption.RequiresNew);
             getBackend().runInternalAction(VdcActionType.UpdateVmVersion, 
params);
         }
         updateVmsJobIdMap.remove(getParameters().getBaseTemplateId());
diff --git a/packaging/dbscripts/vms_sp.sql b/packaging/dbscripts/vms_sp.sql
index 637ed56..0961ba0 100644
--- a/packaging/dbscripts/vms_sp.sql
+++ b/packaging/dbscripts/vms_sp.sql
@@ -507,17 +507,21 @@
     v_is_spice_copy_paste_enabled BOOLEAN)
   RETURNS VOID
    AS $procedure$
+DECLARE
+  v_val  UUID;
 BEGIN
+-- lock template for child count update
+select vm_guid into v_val FROM vm_static WHERE vm_guid = v_vmt_guid for update;
 INSERT INTO vm_static(description, free_text_comment, mem_size_mb, os, 
vds_group_id, vm_guid, VM_NAME, vmt_guid,creation_date,num_of_monitors, 
single_qxl_pci, 
allow_console_reconnect,is_initialized,num_of_sockets,cpu_per_socket,usb_policy,
 time_zone,auto_startup,is_stateless,dedicated_vm_for_vds, fail_back, 
default_boot_sequence, vm_type, nice_level, cpu_shares, default_display_type, 
priority,iso_path,origin,initrd_url,kernel_url,kernel_params,migration_support,predefined_properties,userdefined_properties,min_allocated_mem,
 entity_type, quota_id, cpu_pinning, is_smartcard_enabled,is_delete_protected, 
sso_method, host_cpu_flags, tunnel_migration, vnc_keyboard_layout, 
is_run_and_pause, created_by_user_id, instance_type_id, image_type_id, 
original_template_id, original_template_name, migration_downtime, 
template_version_number, serial_number_policy, custom_serial_number, 
is_boot_menu_enabled, numatune_mode, is_spice_file_transfer_enabled, 
is_spice_copy_paste_enabled)
        VALUES(v_description, v_free_text_comment, v_mem_size_mb, v_os, 
v_vds_group_id, v_vm_guid, v_vm_name, v_vmt_guid, v_creation_date, 
v_num_of_monitors,v_single_qxl_pci, v_allow_console_reconnect, 
v_is_initialized, v_num_of_sockets, v_cpu_per_socket, v_usb_policy, 
v_time_zone, v_auto_startup,v_is_stateless,v_dedicated_vm_for_vds,v_fail_back, 
v_default_boot_sequence, v_vm_type, v_nice_level, v_cpu_shares, 
v_default_display_type, 
v_priority,v_iso_path,v_origin,v_initrd_url,v_kernel_url,v_kernel_params,v_migration_support,v_predefined_properties,v_userdefined_properties,v_min_allocated_mem,
 'VM', v_quota_id, v_cpu_pinning, v_is_smartcard_enabled,v_is_delete_protected, 
v_sso_method, v_host_cpu_flags, v_tunnel_migration, v_vnc_keyboard_layout, 
v_is_run_and_pause, v_created_by_user_id, v_instance_type_id, v_image_type_id, 
v_original_template_id, v_original_template_name, v_migration_downtime, 
v_template_version_number, v_serial_number_policy, v_custom_serial_number, 
v_is_boot_menu_!
 enabled, v_numatune_mode, v_is_spice_file_transfer_enabled, 
v_is_spice_copy_paste_enabled);
 
 -- perform deletion from vm_ovf_generations to ensure that no record exists 
when performing insert to avoid PK violation.
 DELETE FROM vm_ovf_generations gen WHERE gen.vm_guid = v_vm_guid;
 INSERT INTO vm_ovf_generations(vm_guid, storage_pool_id) VALUES (v_vm_guid, 
(SELECT storage_pool_id FROM vds_groups vg WHERE vg.vds_group_id = 
v_vds_group_id));
+
 -- set child_count for the template
-UPDATE vm_static
-    SET child_count =(SELECT COUNT(*) FROM vm_static WHERE vmt_guid = 
v_vmt_guid and entity_type = 'VM')
-    WHERE vm_guid = v_vmt_guid;
+UPDATE vm_static SET child_count = child_count+1 where vm_guid = v_vmt_guid;
+
 END; $procedure$
 LANGUAGE plpgsql;
 
@@ -708,8 +712,7 @@
 
       -- set the child_count for the template
       UPDATE vm_static
-          SET child_count =(SELECT COUNT(*) FROM vm_static WHERE vmt_guid = 
v_vmt_guid and entity_type = 'VM')
-          WHERE vm_guid = v_vmt_guid;
+          SET child_count = child_count - 1 WHERE vm_guid = v_vmt_guid;
 END; $procedure$
 LANGUAGE plpgsql;
 


-- 
To view, visit http://gerrit.ovirt.org/28861
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2dfdb556e66658c3ef630cf71fba5f902ea26862
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Omer Frenkel <ofren...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to