Arik Hadas has uploaded a new change for review.

Change subject: core: increase AutoStartVmsRunner job frequency
......................................................................

core: increase AutoStartVmsRunner job frequency

This patch changes the default frequency of the AutoStartVmsRunner from
60 sec to 1 sec. Instead of invoking the RunVmCommand on each iteration,
the AutoStartVmsRunner now try to acquire the needed VM lock first, and
only if it succeed it invokes the RunVmCommand and the command release
the acquired lock.

The type of the internal collection of HA VMs to run in
AutoStartVmsRunner job is also changed to CopyOnWriteArraySet which is a
more appropriate data structure for it and for the following
planned enhancements.

Change-Id: I0968cae554458f53a1f30643848bb8d88bd4de11
Signed-off-by: Arik Hadas <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AutoStartVmsRunner.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/Backend.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/job/ExecutionHandler.java
M 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
M packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql
5 files changed, 65 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/99/19499/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AutoStartVmsRunner.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AutoStartVmsRunner.java
index 925f677..493fa1a 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AutoStartVmsRunner.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AutoStartVmsRunner.java
@@ -1,17 +1,21 @@
 package org.ovirt.engine.core.bll;
 
-import java.util.concurrent.ConcurrentLinkedQueue;
+import java.util.Collections;
+import java.util.LinkedList;
+import java.util.concurrent.CopyOnWriteArraySet;
 
 import org.ovirt.engine.core.bll.job.ExecutionHandler;
 import org.ovirt.engine.core.common.AuditLogType;
 import org.ovirt.engine.core.common.action.RunVmParams;
 import org.ovirt.engine.core.common.action.VdcActionType;
-import org.ovirt.engine.core.common.action.VdcReturnValueBase;
-import org.ovirt.engine.core.common.utils.Pair;
-import org.ovirt.engine.core.compat.DateTime;
+import org.ovirt.engine.core.common.errors.VdcBllMessages;
+import org.ovirt.engine.core.common.locks.LockingGroup;
 import org.ovirt.engine.core.compat.Guid;
 import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector;
 import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogableBase;
+import org.ovirt.engine.core.utils.lock.EngineLock;
+import org.ovirt.engine.core.utils.lock.LockManager;
+import org.ovirt.engine.core.utils.lock.LockManagerFactory;
 import org.ovirt.engine.core.utils.log.Log;
 import org.ovirt.engine.core.utils.log.LogFactory;
 import org.ovirt.engine.core.utils.timer.OnTimerMethodAnnotation;
@@ -20,7 +24,7 @@
 
     private static Log log = LogFactory.getLog(AutoStartVmsRunner.class);
     private static AutoStartVmsRunner instance = new AutoStartVmsRunner();
-    private ConcurrentLinkedQueue<Pair<DateTime, Guid>> autoStartVmsToRun = 
new ConcurrentLinkedQueue<>();
+    private CopyOnWriteArraySet<Guid> autoStartVmsToRun = new 
CopyOnWriteArraySet<>();
 
     public static AutoStartVmsRunner getInstance() {
         return instance;
@@ -31,27 +35,53 @@
 
     @OnTimerMethodAnnotation("startFailedAutoStartVms")
     public void startFailedAutoStartVms() {
-        DateTime now = DateTime.getNow();
-        while (autoStartVmsToRun.peek() != null && 
now.compareTo(autoStartVmsToRun.peek().getFirst()) > 0) {
-            runVm(autoStartVmsToRun.poll().getSecond());
+        LinkedList<Guid> idsToRemove = new LinkedList<>();
+
+        for(Guid vmId: autoStartVmsToRun) {
+            EngineLock runVmLock = createLockForRunVmCommand(vmId);
+
+            if (!getLockManager().acquireLock(runVmLock).getFirst()) {
+                continue;
+            }
+
+            runVm(vmId, runVmLock);
+
+            idsToRemove.add(vmId);
         }
+
+        autoStartVmsToRun.removeAll(idsToRemove);
+    }
+
+    private EngineLock createLockForRunVmCommand(Guid vmId) {
+        return new EngineLock(
+                Collections.singletonMap(
+                        vmId.toString(),
+                        LockMessagesMatchUtil.makeLockingPair(
+                                LockingGroup.VM,
+                                
VdcBllMessages.ACTION_TYPE_FAILED_OBJECT_LOCKED.name())),
+                null);
+    }
+
+    protected LockManager getLockManager() {
+        return LockManagerFactory.getLockManager();
     }
 
     public void addVmToRun(Guid vmId) {
-        autoStartVmsToRun.add(new Pair<>(DateTime.getNow(), vmId));
+        autoStartVmsToRun.add(vmId);
     }
 
-    private void runVm(Guid vmId) {
-        final VdcReturnValueBase result = 
Backend.getInstance().runInternalAction(VdcActionType.RunVm,
+    private boolean runVm(Guid vmId, EngineLock lock) {
+        boolean succeeded = Backend.getInstance().runInternalAction(
+                VdcActionType.RunVm,
                 new RunVmParams(vmId),
-                ExecutionHandler.createInternalJobContext());
+                
ExecutionHandler.createInternalJobContext(lock)).getSucceeded();
 
-        // Alert if the restart fails:
-        if (!result.getSucceeded()) {
+        if (!succeeded) {
             final AuditLogableBase event = new AuditLogableBase();
             event.setVmId(vmId);
             AuditLogDirector.log(event, AuditLogType.HA_VM_RESTART_FAILED);
-            // should insert to autoStartVmsToRun again?
         }
+
+        return succeeded;
     }
 }
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/Backend.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/Backend.java
index d23bb24..861b18b 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/Backend.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/Backend.java
@@ -98,7 +98,6 @@
     private DateTime _startedAt;
     private static boolean firstInitialization = true;
     private String poolMonitoringJobId;
-    private String autoStartVmsRunnerJobId;
 
     public static BackendInternal getInstance() {
         return EjbUtils.findBean(BeanType.BACKEND, BeanProxyType.LOCAL);
@@ -238,11 +237,10 @@
                         vmPoolMonitorIntervalInMinutes, TimeUnit.MINUTES);
 
         int autoStartVmsRunnerIntervalInSeconds = Config.<Integer> 
GetValue(ConfigValues.AutoStartVmsRunnerIntervalInSeconds);
-        autoStartVmsRunnerJobId =
-                
SchedulerUtilQuartzImpl.getInstance().scheduleAFixedDelayJob(AutoStartVmsRunner.getInstance(),
-                        "startFailedAutoStartVms", new Class[] {}, new 
Object[] {},
-                        autoStartVmsRunnerIntervalInSeconds,
-                        autoStartVmsRunnerIntervalInSeconds, TimeUnit.SECONDS);
+        
SchedulerUtilQuartzImpl.getInstance().scheduleAFixedDelayJob(AutoStartVmsRunner.getInstance(),
+                "startFailedAutoStartVms", new Class[] {}, new Object[] {},
+                autoStartVmsRunnerIntervalInSeconds,
+                autoStartVmsRunnerIntervalInSeconds, TimeUnit.SECONDS);
 
         int quotaCacheIntervalInMinutes = Config.<Integer> 
GetValue(ConfigValues.QuotaCacheIntervalInMinutes);
         
SchedulerUtilQuartzImpl.getInstance().scheduleAFixedDelayJob(QuotaManager.getInstance(),
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/job/ExecutionHandler.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/job/ExecutionHandler.java
index f044bf7..af8a74b 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/job/ExecutionHandler.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/job/ExecutionHandler.java
@@ -488,10 +488,22 @@
      * @return an execution context as a Job
      */
     public static CommandContext createInternalJobContext() {
+        return createInternalJobContext(null);
+    }
+
+    /**
+     * Creates a context for execution of internal command as a monitored Job,
+     * the command will release the given lock when it is finished.
+     *
+     * @param lock
+     *            The lock which should be released at child command (can be 
null)
+     * @return an execution context as a Job
+     */
+    public static CommandContext createInternalJobContext(EngineLock lock) {
         ExecutionContext executionContext = new ExecutionContext();
         executionContext.setJobRequired(true);
         executionContext.setMonitored(true);
-        return new CommandContext(executionContext);
+        return new CommandContext(executionContext, lock);
     }
 
     /**
diff --git 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
index 8fdcc4b..1b8a402 100644
--- 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
+++ 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
@@ -1510,7 +1510,7 @@
     CloudInitSupported(537),
 
     @TypeConverterAttribute(Integer.class)
-    @DefaultValueAttribute("60")
+    @DefaultValueAttribute("1")
     AutoStartVmsRunnerIntervalInSeconds(538),
 
     Invalid(65535);
diff --git a/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql 
b/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql
index a02d727..2c8ff5d 100644
--- a/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql
+++ b/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql
@@ -548,7 +548,7 @@
 select fn_db_add_config_value('VmPoolMonitorMaxAttempts','3','general');
 select fn_db_add_config_value('VmPriorityMaxValue','100','general');
 --How often we'll try to run HA VM that we couldn't run before
-select 
fn_db_add_config_value('AutoStartVmsRunnerIntervalInSeconds','60','general');
+select 
fn_db_add_config_value('AutoStartVmsRunnerIntervalInSeconds','1','general');
 --Handling Keyboard Layout configuration for VNC
 select fn_db_add_config_value('VncKeyboardLayout','en-us','general');
 select 
fn_db_add_config_value('VncKeyboardLayoutValidValues','ar,da,de,de-ch,en-gb,en-us,es,et,fi,fo,fr,fr-be,fr-ca,fr-ch,hr,hu,is,it,ja,lt,lv,mk,nl,nl-be,no,pl,pt,pt-br,ru,sl,sv,th,tr','general');
@@ -729,6 +729,7 @@
 select fn_db_update_config_value('VM64BitMaxMemorySizeInMB','2097152','3.1');
 select fn_db_update_config_value('VM64BitMaxMemorySizeInMB','2097152','3.2');
 select fn_db_update_config_value('VM64BitMaxMemorySizeInMB','2097152','3.3');
+select 
fn_db_update_config_value('AutoStartVmsRunnerIntervalInSeconds','1','general');
 
 
 
------------------------------------------------------------------------------------


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I0968cae554458f53a1f30643848bb8d88bd4de11
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to