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