Hello Eli Mesika, Omer Frenkel, Arik Hadas,

I'd like you to do a code review.  Please visit

    https://gerrit.ovirt.org/41040

to review the following change.

Change subject: core: Inject AutoStartVmsRunner and make it a BackendService
......................................................................

core: Inject AutoStartVmsRunner and make it a BackendService

Change-Id: I5fdf736e8ae33b8b55a6f6294dc0cca4a2603897
Signed-off-by: Roy Golan <rgo...@redhat.com>
---
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/ClearNonResponsiveVdsVmsCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestartVdsVmsOperation.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsEventListener.java
5 files changed, 43 insertions(+), 19 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/40/41040/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 7c3216c..a8f2213 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
@@ -5,9 +5,11 @@
 import java.util.LinkedList;
 import java.util.List;
 import java.util.concurrent.CopyOnWriteArraySet;
+import java.util.concurrent.TimeUnit;
 
 import org.ovirt.engine.core.bll.job.ExecutionHandler;
 import org.ovirt.engine.core.common.AuditLogType;
+import org.ovirt.engine.core.common.BackendService;
 import org.ovirt.engine.core.common.action.RunVmParams;
 import org.ovirt.engine.core.common.action.VdcActionType;
 import org.ovirt.engine.core.common.businessentities.VM;
@@ -24,30 +26,36 @@
 import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogableBase;
 import org.ovirt.engine.core.dao.VmDAO;
 import org.ovirt.engine.core.dao.VmDynamicDAO;
+import org.ovirt.engine.core.di.qualifier.InMemory;
 import org.ovirt.engine.core.utils.lock.EngineLock;
 import org.ovirt.engine.core.utils.lock.LockManagerFactory;
 import org.ovirt.engine.core.utils.timer.OnTimerMethodAnnotation;
+import org.ovirt.engine.core.utils.timer.SchedulerUtil;
+import org.ovirt.engine.core.utils.timer.SchedulerUtilQuartzImpl;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.inject.Singleton;
 
 /**
  * This class represent a job which is responsible for running HA VMs
  */
-public class AutoStartVmsRunner {
+@Singleton
+public class AutoStartVmsRunner implements BackendService{
 
     private static final Logger log = 
LoggerFactory.getLogger(AutoStartVmsRunner.class);
     /** How long to wait before rerun HA VM that failed to start (not because 
of lock acquisition) */
     private static final int RETRY_TO_RUN_HA_VM_INTERVAL =
             Config.<Integer> 
getValue(ConfigValues.RetryToRunAutoStartVmIntervalInSeconds);
-    private static AutoStartVmsRunner instance = new AutoStartVmsRunner();
-    private final AuditLogDirector auditLogDirector = new AuditLogDirector();
+    @Inject
+    private AuditLogDirector auditLogDirector;
+    @Inject @InMemory
+    private SchedulerUtil schedulerUtil;
 
     /** Records of HA VMs that need to be restarted */
     private CopyOnWriteArraySet<AutoStartVmToRestart> autoStartVmsToRestart;
-
-    public static AutoStartVmsRunner getInstance() {
-        return instance;
-    }
 
     private AutoStartVmsRunner() {
         // There might be HA VMs which went down just before the engine 
stopped, we detected
@@ -63,6 +71,20 @@
         autoStartVmsToRestart = new CopyOnWriteArraySet<>(initialFailedVms);
     }
 
+    @PostConstruct
+    private void init() {
+        int autoStartVmsRunnerIntervalInSeconds =
+                
Config.<Integer>getValue(ConfigValues.AutoStartVmsRunnerIntervalInSeconds);
+        SchedulerUtilQuartzImpl.getInstance().scheduleAFixedDelayJob(
+                this,
+                "startFailedAutoStartVms",
+                new Class[] {},
+                new Object[] {},
+                autoStartVmsRunnerIntervalInSeconds,
+                autoStartVmsRunnerIntervalInSeconds,
+                TimeUnit.SECONDS);
+    }
+
     /**
      * Add the given VM IDs to the set of VMs which will be started in the 
next iteration.
      *
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 a58ac91..aa91d18 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
@@ -261,13 +261,7 @@
         _startedAt = DateTime.getNow();
 
         loadService(VmPoolMonitor.class);
-
-        int autoStartVmsRunnerIntervalInSeconds =
-                
Config.<Integer>getValue(ConfigValues.AutoStartVmsRunnerIntervalInSeconds);
-        
SchedulerUtilQuartzImpl.getInstance().scheduleAFixedDelayJob(AutoStartVmsRunner.getInstance(),
-                "startFailedAutoStartVms", new Class[] {}, new Object[] {},
-                autoStartVmsRunnerIntervalInSeconds,
-                autoStartVmsRunnerIntervalInSeconds, TimeUnit.SECONDS);
+        loadService(AutoStartVmsRunner.class);
 
         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/ClearNonResponsiveVdsVmsCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ClearNonResponsiveVdsVmsCommand.java
index b8f8a95..db16404 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ClearNonResponsiveVdsVmsCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ClearNonResponsiveVdsVmsCommand.java
@@ -21,8 +21,13 @@
 import org.ovirt.engine.core.common.vdscommands.VDSReturnValue;
 import org.ovirt.engine.core.compat.Guid;
 
+import javax.inject.Inject;
+
 @NonTransactiveCommandAttribute
 public class ClearNonResponsiveVdsVmsCommand<T extends VdsActionParameters> 
extends VdsCommand<T> {
+
+    @Inject
+    private AutoStartVmsRunner autoStartVmsRunner;
 
     /**
      * Constructor for command creation when compensation is applied on startup
@@ -68,7 +73,7 @@
         runVdsCommand(VDSCommandType.UpdateVdsVMsCleared,
                         new 
UpdateVdsVMsClearedVDSCommandParameters(getVdsId()));
         if (!autoStartVmIdsToRerun.isEmpty()) {
-            
AutoStartVmsRunner.getInstance().addVmsToRun(autoStartVmIdsToRerun);
+            autoStartVmsRunner.addVmsToRun(autoStartVmIdsToRerun);
         }
         setSucceeded(true);
     }
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestartVdsVmsOperation.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestartVdsVmsOperation.java
index 99522f1..d759b01 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestartVdsVmsOperation.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestartVdsVmsOperation.java
@@ -19,6 +19,7 @@
 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.di.Injector;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -128,7 +129,7 @@
         }
 
         if (!autoStartVmIdsToRerun.isEmpty()) {
-            
AutoStartVmsRunner.getInstance().addVmsToRun(autoStartVmIdsToRerun);
+            
Injector.get(AutoStartVmsRunner.class).addVmsToRun(autoStartVmIdsToRerun);
         }
     }
 }
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsEventListener.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsEventListener.java
index da08ed1..36f560a 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsEventListener.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsEventListener.java
@@ -86,9 +86,11 @@
     Instance<ResourceManager> resourceManagerProvider;
     @Inject
     EventQueue eventQueue;
-
     @Inject
     private AvailableUpdatesFinder availableUpdatesFinder;
+    @Inject
+    AutoStartVmsRunner autoStartVmsRunner;
+
 
     private static final Logger log = 
LoggerFactory.getLogger(VdsEventListener.class);
 
@@ -144,7 +146,7 @@
      */
     private void clearDomainCache(final VDS vds) {
         eventQueue.submitEventSync(new Event(vds.getStoragePoolId(),
-                null, vds.getId(), EventType.VDSCLEARCACHE, ""),
+                        null, vds.getId(), EventType.VDSCLEARCACHE, ""),
                 new Callable<EventResult>() {
                     @Override
                     public EventResult call() {
@@ -405,7 +407,7 @@
                     event.getVmName(), vmId);
         }
 
-        AutoStartVmsRunner.getInstance().addVmsToRun(vmIds);
+        autoStartVmsRunner.addVmsToRun(vmIds);
     }
 
     @Override


-- 
To view, visit https://gerrit.ovirt.org/41040
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5fdf736e8ae33b8b55a6f6294dc0cca4a2603897
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: 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