Michael Kublin has uploaded a new change for review. Change subject: engine: Preventing running host monitor during activation of host operation ......................................................................
engine: Preventing running host monitor during activation of host operation The following patch will prevent running of host monitoring during Activate host operation. It is means that activate host operation will wait untill monitoring will be finished and will acquire a lock. When a lock is acquired by activate operation a monitoring will be skipped. Change-Id: If7f5eed741ca392ed7144934c539d2aa70744431 Signed-off-by: Michael Kublin <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ActivateVdsCommand.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/locks/LockingGroup.java M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManager.java 3 files changed, 118 insertions(+), 94 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/85/11885/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ActivateVdsCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ActivateVdsCommand.java index 2a98cf2..d16e9b0 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ActivateVdsCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ActivateVdsCommand.java @@ -19,6 +19,7 @@ import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dal.VdcBllMessages; import org.ovirt.engine.core.dal.dbbroker.DbFacade; +import org.ovirt.engine.core.utils.lock.EngineLock; import org.ovirt.engine.core.utils.transaction.TransactionMethod; import org.ovirt.engine.core.utils.transaction.TransactionSupport; @@ -50,37 +51,47 @@ @Override protected void executeCommand() { - final VDS vds = getVds(); - ExecutionHandler.updateSpecificActionJobCompleted(vds.getId(), VdcActionType.MaintananceVds, false); - TransactionSupport.executeInNewTransaction(new TransactionMethod<Void>() { - - @Override - public Void runInTransaction() { - getCompensationContext().snapshotEntityStatus(vds.getDynamicData(), vds.getstatus()); - Backend.getInstance().getResourceManager().RunVdsCommand(VDSCommandType.SetVdsStatus, - new SetVdsStatusVDSCommandParameters(getVdsId(), VDSStatus.Unassigned)); - getCompensationContext().stateChanged(); - return null; - } - }); - - setSucceeded(Backend.getInstance().getResourceManager() - .RunVdsCommand(VDSCommandType.ActivateVds, new ActivateVdsVDSCommandParameters(getVdsId())) - .getSucceeded()); - if (getSucceeded()) { + EngineLock monitoring_lock = + new EngineLock(Collections.singletonMap(getParameters().getVdsId().toString(), + LockingGroup.VDS_INIT.name()), null); + log.info("Before acquiring lock in order to prevent monitoring"); + getLockManager().acquireLockWait(monitoring_lock); + log.info("Lock acquired, from now a monitoring of host will be skipped"); + try { + final VDS vds = getVds(); + ExecutionHandler.updateSpecificActionJobCompleted(vds.getId(), VdcActionType.MaintananceVds, false); TransactionSupport.executeInNewTransaction(new TransactionMethod<Void>() { @Override public Void runInTransaction() { - // set network to operational / non-operational - List<Network> networks = DbFacade.getInstance().getNetworkDao() - .getAllForCluster(vds.getvds_group_id()); - for (Network net : networks) { - NetworkClusterHelper.setStatus(vds.getvds_group_id(), net); - } + getCompensationContext().snapshotEntityStatus(vds.getDynamicData(), vds.getstatus()); + runVdsCommand(VDSCommandType.SetVdsStatus, + new SetVdsStatusVDSCommandParameters(getVdsId(), VDSStatus.Unassigned)); + getCompensationContext().stateChanged(); return null; } }); + + setSucceeded(runVdsCommand(VDSCommandType.ActivateVds, new ActivateVdsVDSCommandParameters(getVdsId())) + .getSucceeded()); + if (getSucceeded()) { + TransactionSupport.executeInNewTransaction(new TransactionMethod<Void>() { + + @Override + public Void runInTransaction() { + // set network to operational / non-operational + List<Network> networks = DbFacade.getInstance().getNetworkDao() + .getAllForCluster(vds.getvds_group_id()); + for (Network net : networks) { + NetworkClusterHelper.setStatus(vds.getvds_group_id(), net); + } + return null; + } + }); + } + } finally { + getLockManager().releaseLock(monitoring_lock); + log.info("Activate finished. Lock released. Monitoring can run now"); } } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/locks/LockingGroup.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/locks/LockingGroup.java index 8238a14..e649278 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/locks/LockingGroup.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/locks/LockingGroup.java @@ -4,6 +4,7 @@ POOL, VDS, + VDS_INIT, VDS_FENCE, VM, TEMPLATE, 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 dea1028..9eb20af 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 @@ -1,6 +1,7 @@ package org.ovirt.engine.core.vdsbroker; import java.util.ArrayList; +import java.util.Collections; import java.util.HashSet; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -21,6 +22,7 @@ import org.ovirt.engine.core.common.businessentities.VmDynamic; import org.ovirt.engine.core.common.config.Config; import org.ovirt.engine.core.common.config.ConfigValues; +import org.ovirt.engine.core.common.locks.LockingGroup; import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.common.vdscommands.SetVdsStatusVDSCommandParameters; import org.ovirt.engine.core.common.vdscommands.VDSCommandType; @@ -32,6 +34,8 @@ import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector; import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogableBase; import org.ovirt.engine.core.utils.FileUtil; +import org.ovirt.engine.core.utils.lock.EngineLock; +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; @@ -70,6 +74,7 @@ private static Map<Guid, String> recoveringJobIdMap = new ConcurrentHashMap<Guid, String>(); private boolean isSetNonOperationalExecuted; private MonitoringStrategy monitoringStrategy; + private EngineLock monitoring_lock; public Object getLockObj() { return _lockObj; } @@ -131,6 +136,8 @@ monitoringStrategy = MonitoringStrategyFactory.getMonitoringStrategyForVds(vds); mUnrespondedAttempts = new AtomicInteger(); mFailedToRunVmAttempts = new AtomicInteger(); + monitoring_lock = new EngineLock(Collections.singletonMap(_vdsId.toString(), + LockingGroup.VDS_INIT.name()), null); if (_vds.getstatus() == VDSStatus.PreparingForMaintenance) { _vds.setprevious_status(_vds.getstatus()); @@ -139,7 +146,7 @@ } // if ssl is on and no certificate file if (Config.<Boolean> GetValue(ConfigValues.UseSecureConnectionWithServers) - && !FileUtil.fileExists(Config.resolveCertificatePath())) { + && !FileUtil.fileExists(Config.resolveCertificatePath())) { if (_vds.getstatus() != VDSStatus.Maintenance && _vds.getstatus() != VDSStatus.InstallFailed) { setStatus(VDSStatus.NonResponsive, _vds); UpdateDynamicData(_vds.getDynamicData()); @@ -192,85 +199,90 @@ @OnTimerMethodAnnotation("OnTimer") public void OnTimer() { - try { - setIsSetNonOperationalExecuted(false); - Guid vdsId = null; - Guid storagePoolId = null; - String vdsName = null; - ArrayList<VDSDomainsData> domainsList = null; + if (LockManagerFactory.getLockManager().acquireLock(monitoring_lock)) { + try { + setIsSetNonOperationalExecuted(false); + Guid vdsId = null; + Guid storagePoolId = null; + String vdsName = null; + ArrayList<VDSDomainsData> domainsList = null; - synchronized (getLockObj()) { - _vds = DbFacade.getInstance().getVdsDao().get(getVdsId()); - if (_vds == null) { - log.errorFormat("VdsManager::refreshVdsRunTimeInfo - OnTimer is NULL for {0}", - getVdsId()); - return; - } + synchronized (getLockObj()) { + _vds = DbFacade.getInstance().getVdsDao().get(getVdsId()); + if (_vds == null) { + log.errorFormat("VdsManager::refreshVdsRunTimeInfo - OnTimer is NULL for {0}", + getVdsId()); + return; + } - try { - if (_refreshIteration == _numberRefreshesBeforeSave) { - _refreshIteration = 1; - } else { - _refreshIteration++; - } - if (isMonitoringNeeded()) { - setStartTime(); - _vdsUpdater = new VdsUpdateRunTimeInfo(VdsManager.this, _vds); - _vdsUpdater.Refresh(); - mUnrespondedAttempts.set(0); - setLastUpdate(); - } - if (!getInitialized() && _vds.getstatus() != VDSStatus.NonResponsive - && _vds.getstatus() != VDSStatus.PendingApproval) { - log.infoFormat("Initializing Host: {0}", _vds.getvds_name()); - ResourceManager.getInstance().HandleVdsFinishedInit(_vds.getId()); - setInitialized(true); - } - } catch (VDSNetworkException e) { - logNetworkException(e); - } catch (VDSRecoveringException ex) { - HandleVdsRecoveringException(ex); - } catch (IRSErrorException ex) { - logFailureMessage(ex); - } catch (RuntimeException ex) { - logFailureMessage(ex); - } - try { - if (_vdsUpdater != null) { - _vdsUpdater.AfterRefreshTreatment(); - - // Get vds data for updating domains list, ignoring vds which is down, since it's not connected - // to - // the storage anymore (so there is no sense in updating the domains list in that case). - if (_vds != null && _vds.getstatus() != VDSStatus.Maintenance) { - vdsId = _vds.getId(); - vdsName = _vds.getvds_name(); - storagePoolId = _vds.getStoragePoolId(); - domainsList = _vds.getDomains(); + try { + if (_refreshIteration == _numberRefreshesBeforeSave) { + _refreshIteration = 1; + } else { + _refreshIteration++; } + if (isMonitoringNeeded()) { + setStartTime(); + _vdsUpdater = new VdsUpdateRunTimeInfo(VdsManager.this, _vds); + _vdsUpdater.Refresh(); + mUnrespondedAttempts.set(0); + setLastUpdate(); + } + if (!getInitialized() && _vds.getstatus() != VDSStatus.NonResponsive + && _vds.getstatus() != VDSStatus.PendingApproval) { + log.infoFormat("Initializing Host: {0}", _vds.getvds_name()); + ResourceManager.getInstance().HandleVdsFinishedInit(_vds.getId()); + setInitialized(true); + } + } catch (VDSNetworkException e) { + logNetworkException(e); + } catch (VDSRecoveringException ex) { + HandleVdsRecoveringException(ex); + } catch (IRSErrorException ex) { + logFailureMessage(ex); + } catch (RuntimeException ex) { + logFailureMessage(ex); } + try { + if (_vdsUpdater != null) { + _vdsUpdater.AfterRefreshTreatment(); - _vds = null; - _vdsUpdater = null; - } catch (IRSErrorException ex) { - logAfterRefreshFailureMessage(ex); - if (log.isDebugEnabled()) { + // Get vds data for updating domains list, ignoring vds which is down, since it's not + // connected + // to + // the storage anymore (so there is no sense in updating the domains list in that case). + if (_vds != null && _vds.getstatus() != VDSStatus.Maintenance) { + vdsId = _vds.getId(); + vdsName = _vds.getvds_name(); + storagePoolId = _vds.getStoragePoolId(); + domainsList = _vds.getDomains(); + } + } + + _vds = null; + _vdsUpdater = null; + } catch (IRSErrorException ex) { + logAfterRefreshFailureMessage(ex); + if (log.isDebugEnabled()) { + logException(ex); + } + } catch (RuntimeException ex) { + logAfterRefreshFailureMessage(ex); logException(ex); } - } catch (RuntimeException ex) { - logAfterRefreshFailureMessage(ex); - logException(ex); + } + // Now update the status of domains, this code should not be in + // synchronized part of code + if (domainsList != null) { + IrsBrokerCommand.UpdateVdsDomainsData(vdsId, vdsName, storagePoolId, domainsList); + } + } catch (Exception e) { + log.error("Timer update runtimeinfo failed. Exception:", e); + } finally { + LockManagerFactory.getLockManager().releaseLock(monitoring_lock); } - - // Now update the status of domains, this code should not be in - // synchronized part of code - if (domainsList != null) { - IrsBrokerCommand.UpdateVdsDomainsData(vdsId, vdsName, storagePoolId, domainsList); - } - } catch (Exception e) { - log.error("Timer update runtimeinfo failed. Exception:", e); } } -- To view, visit http://gerrit.ovirt.org/11885 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: If7f5eed741ca392ed7144934c539d2aa70744431 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Michael Kublin <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
