Liron Aravot has uploaded a new change for review. Change subject: core: determine whether domain monitoring result is actual ......................................................................
core: determine whether domain monitoring result is actual When the domain monitoring results are reported back from vdsm, the first monitor run may haven't finished yet. In that case the returned status from vdsm is a dummy status and not an actual result. The non actual status could lead to a wrong decision making on the engine status (for example - decide that an inactive domain is now active). This patch uses the logic added on vdsm side (change I1fea5) to inspect whether the reported status is actual or not. For earlier version that doesn't return that info, each returned report will be considered as actual to leave the behavior as it was. Change-Id: Iddbba94d81694bd6cb1ce3648872eb81182a50ab Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1183977 Signed-off-by: lara...@redhat.com <lara...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitVdsOnUpCommand.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/FeatureSupported.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDSDomainsData.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsProxyData.java M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsProperties.java M packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql 9 files changed, 65 insertions(+), 21 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/42/38842/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitVdsOnUpCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitVdsOnUpCommand.java index 892f5ba..3f3f3e8 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitVdsOnUpCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitVdsOnUpCommand.java @@ -273,7 +273,7 @@ } if (result.isSuccess()) { - Pair<Boolean, List<StorageDomainStatic>> vdsStatsResults = proceedVdsStats(!masterDomainInactiveOrUnknown); + Pair<Boolean, List<StorageDomainStatic>> vdsStatsResults = proceedVdsStats(!masterDomainInactiveOrUnknown, storagePool); result.setSuccess(vdsStatsResults.getFirst()); if (!result.isSuccess()) { result.setResultData(vdsStatsResults.getSecond()); @@ -300,14 +300,13 @@ return returnValue; } - private Pair<Boolean, List<StorageDomainStatic>> proceedVdsStats(boolean shouldCheckReportedDomains) { + private Pair<Boolean, List<StorageDomainStatic>> proceedVdsStats(boolean shouldCheckReportedDomains, StoragePool storagePool) { Pair<Boolean, List<StorageDomainStatic>> returnValue = new Pair<>(true, null); try { runVdsCommand(VDSCommandType.GetStats, new VdsIdAndVdsVDSCommandParametersBase(getVds())); if (shouldCheckReportedDomains) { List<Guid> problematicDomainsIds = - IrsBrokerCommand.fetchDomainsReportedAsProblematic(getVds().getStoragePoolId(), - getVds().getDomains()); + IrsBrokerCommand.fetchDomainsReportedAsProblematic(getVds().getDomains(), storagePool); for (Guid domainId : problematicDomainsIds) { StorageDomainStatic domainInfo = getStorageDomainStaticDAO().get(domainId); log.errorFormat("Storage Domain {0} of pool {1} is in problem in host {2}", diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/FeatureSupported.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/FeatureSupported.java index bccc0e6..fb53456 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/FeatureSupported.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/FeatureSupported.java @@ -442,4 +442,12 @@ return supportedInConfig(ConfigValues.JsonProtocolSupported, version); } + /** + * @param version + * Compatibility version to check for. + * @return <code>true</code> if report whether domain monitoring result is supported for the given version. + */ + public static boolean reportWhetherDomainMonitoringResultIsActual(Version version) { + return supportedInConfig(ConfigValues.ReportWhetherDomainMonitoringResultIsActual, version); + } } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDSDomainsData.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDSDomainsData.java index e72b64c..fc44a86 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDSDomainsData.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDSDomainsData.java @@ -9,6 +9,7 @@ private Guid privateDomainId; private double lastCheck; private double delay; + private boolean actual; public Guid getDomainId() { return privateDomainId; @@ -44,6 +45,14 @@ return delay; } + public boolean isActual() { + return actual; + } + + public void setActual(boolean actual) { + this.actual = actual; + } + public VDSDomainsData() { privateDomainId = Guid.Empty; } 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 977aa58..2a84470 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 @@ -2010,5 +2010,9 @@ @OptionBehaviourAttribute(behaviour = OptionBehaviour.CommaSeparatedStringArray) UnsupportedLocalesFilter, + @TypeConverterAttribute(Boolean.class) + @DefaultValueAttribute("true") + ReportWhetherDomainMonitoringResultIsActual, + Invalid; } diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java index ec1073e..4124e86 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java +++ b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java @@ -65,10 +65,10 @@ } } - public static List<Guid> fetchDomainsReportedAsProblematic(Guid storagePoolId, List<VDSDomainsData> vdsDomainsData) { - IrsProxyData proxy = _irsProxyData.get(storagePoolId); + public static List<Guid> fetchDomainsReportedAsProblematic(List<VDSDomainsData> vdsDomainsData, StoragePool storagePool) { + IrsProxyData proxy = _irsProxyData.get(storagePool.getId()); if (proxy != null) { - return proxy.obtainDomainsReportedAsProblematic(vdsDomainsData); + return proxy.obtainDomainsReportedAsProblematic(vdsDomainsData, storagePool); } return Collections.emptyList(); } diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsProxyData.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsProxyData.java index 6da6e7ba..92c54d0 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsProxyData.java +++ b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsProxyData.java @@ -17,6 +17,7 @@ import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.common.AuditLogType; +import org.ovirt.engine.core.common.FeatureSupported; import org.ovirt.engine.core.common.businessentities.AsyncTaskStatus; import org.ovirt.engine.core.common.businessentities.AsyncTaskStatusEnum; import org.ovirt.engine.core.common.businessentities.BusinessEntitiesDefinitions; @@ -1106,14 +1107,14 @@ // Unknown domains in pool for (VDSDomainsData tempData : data) { if (domainsInPool.contains(tempData.getDomainId())) { - DomainMonitoringResult domainMonitoringResult = analyzeDomainReport(tempData, false); - if (domainMonitoringResult.invalid()) { + DomainMonitoringResult domainMonitoringResult = analyzeDomainReport(tempData, storagePool, false); + if (domainMonitoringResult.invalidAndActual()) { domainsProblematicReportInfo.put(tempData.getDomainId(), domainMonitoringResult); - } else if (tempData.getDelay() > Config.<Double> getValue(ConfigValues.MaxStorageVdsDelayCheckSec)) { + } else if (domainMonitoringResult.actual() && tempData.getDelay() > Config.<Double> getValue(ConfigValues.MaxStorageVdsDelayCheckSec)) { logDelayedDomain(vdsId, tempData); } } else if (inActiveDomainsInPool.contains(tempData.getDomainId()) - && analyzeDomainReport(tempData, false).valid()) { + && analyzeDomainReport(tempData, storagePool, false).validAndActual()) { log.warnFormat("Storage Domain {0} was reported by Host {1} as Active in Pool {2}, moving to active status", getDomainIdTuple(tempData.getDomainId()), vdsName, @@ -1179,7 +1180,7 @@ AuditLogType.VDS_DOMAIN_DELAY_INTERVAL); } - protected List<Guid> obtainDomainsReportedAsProblematic(List<VDSDomainsData> vdsDomainsData) { + protected List<Guid> obtainDomainsReportedAsProblematic(List<VDSDomainsData> vdsDomainsData, StoragePool storagePool) { List<Guid> domainsInProblem = new LinkedList<>(); Set<Guid> domainsInPool = new HashSet<Guid>( DbFacade.getInstance().getStorageDomainStaticDao().getAllIds( @@ -1189,7 +1190,7 @@ List<Guid> domainWhichWereSeen = new ArrayList<Guid>(); for (VDSDomainsData vdsDomainData : vdsDomainsData) { if (domainsInPool.contains(vdsDomainData.getDomainId())) { - if (analyzeDomainReport(vdsDomainData, true).invalid()) { + if (analyzeDomainReport(vdsDomainData, storagePool, true).invalidAndActual()) { domainsInProblem.add(vdsDomainData.getDomainId()); } domainWhichWereSeen.add(vdsDomainData.getDomainId()); @@ -1206,24 +1207,35 @@ } private enum DomainMonitoringResult { - PROBLEMATIC(false), STORAGE_ACCCESS_ERROR(false), OK(true), NOT_REPORTED(false); + PROBLEMATIC(Boolean.FALSE), STORAGE_ACCCESS_ERROR(Boolean.FALSE), OK(Boolean.TRUE), NOT_REPORTED(Boolean.FALSE), NOT_ACTUAL(null); - private boolean valid; + private Boolean valid; - private DomainMonitoringResult(boolean valid) { + private DomainMonitoringResult(Boolean valid) { this.valid = valid; } - public boolean valid() { - return valid; + public boolean validAndActual() { + return actual() && valid; } - public boolean invalid() { - return !valid; + public boolean invalidAndActual() { + return actual() && !valid; + } + + public boolean actual() { + return this != NOT_ACTUAL; } } - private DomainMonitoringResult analyzeDomainReport(VDSDomainsData tempData, boolean isLog) { + private DomainMonitoringResult analyzeDomainReport(VDSDomainsData tempData, StoragePool storagePool, boolean isLog) { + if (!tempData.isActual() && + FeatureSupported.reportWhetherDomainMonitoringResultIsActual(storagePool.getcompatibility_version())) { + log.warnFormat("Domain {0} report isn't an actual report", + getDomainIdTuple(tempData.getDomainId())); + return DomainMonitoringResult.NOT_ACTUAL; + } + if (tempData.getCode() != 0) { if (isLog) { log.errorFormat("Domain {0} was reported with error code {1}", diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java index 4d24960..9305232 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java +++ b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java @@ -933,6 +933,11 @@ delay = Double.parseDouble((String) internalValue.get(VdsProperties.delay)); } data.setDelay(delay); + Boolean actual = Boolean.TRUE; + if (internalValue.containsKey(VdsProperties.actual)) { + actual = (Boolean)internalValue.get(VdsProperties.actual); + } + data.setActual(actual); domainsData.add(data); } catch (Exception e) { log.error("failed building domains", e); diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsProperties.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsProperties.java index ee82ddf..675c0c9 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsProperties.java +++ b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsProperties.java @@ -360,6 +360,7 @@ public static final String code = "code"; public static final String lastCheck = "lastCheck"; public static final String delay = "delay"; + public static final String actual = "actual"; public static final String DISK_STATS = "diskStats"; public static final String DISK_STATS_FREE = "free"; diff --git a/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql b/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql index 71c27f5..2bb6e50 100644 --- a/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql +++ b/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql @@ -306,6 +306,12 @@ select fn_db_add_config_value('ImportDataStorageDomain','false','3.3'); select fn_db_add_config_value('ImportDataStorageDomain','false','3.4'); +select fn_db_add_config_value('ReportWhetherDomainMonitoringResultIsActual','false','3.0'); +select fn_db_add_config_value('ReportWhetherDomainMonitoringResultIsActual','false','3.1'); +select fn_db_add_config_value('ReportWhetherDomainMonitoringResultIsActual','false','3.2'); +select fn_db_add_config_value('ReportWhetherDomainMonitoringResultIsActual','false','3.3'); +select fn_db_add_config_value('ReportWhetherDomainMonitoringResultIsActual','false','3.4'); + -- Mixed domain types in a data center support select fn_db_add_config_value('MixedDomainTypesInDataCenter','false','3.0'); select fn_db_add_config_value('MixedDomainTypesInDataCenter','false','3.1'); -- To view, visit https://gerrit.ovirt.org/38842 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iddbba94d81694bd6cb1ce3648872eb81182a50ab Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.5.2 Gerrit-Owner: Liron Aravot <lara...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
