Arik Hadas has uploaded a new change for review.

Change subject: core: move cached info about last event to vm-manager
......................................................................

core: move cached info about last event to vm-manager

The caching of the timestamp of the last event we got per-VM is moved to
VmManager instead of being in VdsManager:
1. It is a value per-VM, thus VmManager is more appropriate for that
than having it in VdsManager
2. When saving the host we got the last event from with the timestamp we
don't need to reset the timestamp when the VM is moving between hosts
(in the previous state it looks like we missed the case of rerun for
RunVm for example)

Change-Id: I9ee4470d43e3d4690343a0fdc23df2e1fdddbf8e
Signed-off-by: Arik Hadas <aha...@redhat.com>
---
M 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManager.java
M 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VmAnalyzer.java
M 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VmManager.java
M 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VmsMonitoring.java
4 files changed, 28 insertions(+), 32 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/69/42469/1

diff --git 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManager.java
 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManager.java
index abfd41d..06a4359 100644
--- 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManager.java
+++ 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManager.java
@@ -70,8 +70,7 @@
 
 public class VdsManager {
     private static Logger log = LoggerFactory.getLogger(VdsManager.class);
-    private static Map<Guid, String> recoveringJobIdMap = new 
ConcurrentHashMap<Guid, String>();
-    private final ConcurrentMap<Guid, Double> vmStatusUpdated = new 
ConcurrentHashMap<Guid, Double>();
+    private static Map<Guid, String> recoveringJobIdMap = new 
ConcurrentHashMap<>();
     private final Object lockObj = new Object();
     private final AtomicInteger mFailedToRunVmAttempts;
     private final AtomicInteger mUnrespondedAttempts;
@@ -169,29 +168,6 @@
 
     private SchedulerUtil getSchedulUtil() {
         return Injector.get(SchedulerUtilQuartzImpl.class);
-    }
-
-    public boolean shouldUpdateVmStatus(VmInternalData vmInternalData) {
-        if (vmInternalData == null) {
-            // VM disappeared from VDSM, we need to have monitoring cycle
-            return true;
-        }
-        Guid id = vmInternalData.getVmDynamic().getId();
-        if (!vmStatusUpdated.containsKey(id)) {
-            vmStatusUpdated.put(id, vmInternalData.getTimestamp());
-            return true;
-        }
-        Double knownStatusUpdate = vmStatusUpdated.get(id);
-        Double statusUpdateTime = vmInternalData.getTimestamp();
-        if (knownStatusUpdate <= statusUpdateTime) {
-            vmStatusUpdated.replace(id, statusUpdateTime);
-            return true;
-        }
-        return false;
-    }
-
-    public void resetStatusUpdateTime(Guid id) {
-        vmStatusUpdated.remove(id);
     }
 
     private void initVdsBroker() {
diff --git 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VmAnalyzer.java
 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VmAnalyzer.java
index 8f6c7b8..74e0ff0 100644
--- 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VmAnalyzer.java
+++ 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VmAnalyzer.java
@@ -642,8 +642,6 @@
                         : VMStatus.MigratingTo;
 
         // handing over the VM to the DST by marking it running on it. it will 
now be its SRC host.
-        VdsManager manager = getVdsManager();
-        manager.resetStatusUpdateTime(vmToRemove.getId());
         vmToRemove.setRunOnVds(destinationHostId);
 
         log.info("Handing over VM '{}'({}) to Host '{}'. Setting VM to status 
'{}'",
@@ -694,9 +692,6 @@
         // is not MigratingFrom, it means the migration failed
         if (oldVmStatus == VMStatus.MigratingFrom && currentVmStatus != 
VMStatus.MigratingFrom
                 && currentVmStatus.isRunning()) {
-            VdsManager manager = getVdsManager();
-            manager.resetStatusUpdateTime(vmToUpdate.getId());
-
             rerun = true;
             log.info("Adding VM '{}' to re-run list", runningVm.getId());
             vmToUpdate.setMigratingToVds(null);
diff --git 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VmManager.java
 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VmManager.java
index 90536e5..a1167c7 100644
--- 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VmManager.java
+++ 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VmManager.java
@@ -1,6 +1,7 @@
 package org.ovirt.engine.core.vdsbroker;
 
 import java.util.concurrent.locks.ReentrantLock;
+
 import org.ovirt.engine.core.common.businessentities.VMStatus;
 import org.ovirt.engine.core.common.businessentities.VmDynamic;
 import org.ovirt.engine.core.common.businessentities.VmStatistics;
@@ -12,6 +13,7 @@
 import org.ovirt.engine.core.dao.network.VmNetworkStatisticsDao;
 import org.ovirt.engine.core.utils.transaction.TransactionMethod;
 import org.ovirt.engine.core.utils.transaction.TransactionSupport;
+import org.ovirt.engine.core.vdsbroker.vdsbroker.entities.VmInternalData;
 
 public class VmManager {
 
@@ -21,6 +23,9 @@
 
     private int convertOperationProgress;
     private String convertOperationDescription;
+
+    private Double lastStatusEventTimestamp;
+    private Guid lastStatusEventReporterId;
 
     public VmManager(Guid id) {
         this.id = id;
@@ -107,4 +112,24 @@
     public final void updateVmDataChangedTime() {
         vmDataChangedTime = System.nanoTime();
     }
+
+    /**
+     * Check whether the given data is the latest we got from the given host
+     * @param vmInternalData - the data received
+     * @param vdsId - the host that sent the data
+     * @return false if newer data was already processed, true otherwise
+     */
+    boolean isLatestData(VmInternalData vmInternalData, Guid vdsId) {
+        if (vmInternalData == null) {
+            // VM disappeared from VDSM, we need to have monitoring cycle
+            return true;
+        }
+        Double statusEventTimestamp = vmInternalData.getTimestamp();
+        if (!vdsId.equals(lastStatusEventReporterId) || 
lastStatusEventTimestamp <= statusEventTimestamp) {
+            lastStatusEventTimestamp = statusEventTimestamp;
+            vdsId = lastStatusEventReporterId;
+            return true;
+        }
+        return false;
+    }
 }
diff --git 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VmsMonitoring.java
 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VmsMonitoring.java
index c5e6425..a5cd393 100644
--- 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VmsMonitoring.java
+++ 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VmsMonitoring.java
@@ -10,6 +10,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+
 import org.apache.commons.lang.StringUtils;
 import org.ovirt.engine.core.common.FeatureSupported;
 import org.ovirt.engine.core.common.businessentities.Entities;
@@ -147,8 +148,7 @@
 
         if (vmId != null) {
             VmManager vmManager = getResourceManager().getVmManager(vmId);
-
-            if (vdsManager.shouldUpdateVmStatus(pair.getSecond()) && 
vmManager.trylock()) {
+            if (vmManager.isLatestData(pair.getSecond(), 
vdsManager.getVdsId()) && vmManager.trylock()) {
                 if (fetchTime - vmManager.getVmDataChangedTime() <= 0) {
                     log.warn("skipping VM '{}' from this monitoring cycle" +
                             " - the VM data has changed since fetching the 
data", vmId);


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

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

Reply via email to