Liron Aravot has uploaded a new change for review. Change subject: core: handling of possible race when deactivating domain ......................................................................
core: handling of possible race when deactivating domain When deactivating the last master domain a new thread is started to make sure that the domain is deactivated only after the tasks are cleared. In case of engine crash during that time the domain may remain in status LOCKED, because nothing will restart the thread on the engine start (as the tasks were cleared). The infrastructure that we currently have to handle that case is the compensation context, in this patch i create a compensation context that will assure that the domain will return to the appropiate status in case of an engine crash. Change-Id: If4e482a2a13741e8b4509851d9d8e4645d8b81e6 Signed-off-by: Liron Aravot <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainWithOvfUpdateCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java 3 files changed, 29 insertions(+), 10 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/15/35215/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java index 58513ef..41a9cfe 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java @@ -284,6 +284,10 @@ return NoOpCompensationContext.getInstance(); } + return createDefaultCompensationContext(commandId); + } + + protected DefaultCompensationContext createDefaultCompensationContext(Guid commandId) { DefaultCompensationContext defaultContext = new DefaultCompensationContext(); defaultContext.setCommandId(commandId); defaultContext.setCommandType(getClass().getName()); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainWithOvfUpdateCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainWithOvfUpdateCommand.java index 36b5ca9..39d1dfb 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainWithOvfUpdateCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainWithOvfUpdateCommand.java @@ -6,6 +6,7 @@ import org.ovirt.engine.core.bll.NonTransactiveCommandAttribute; import org.ovirt.engine.core.bll.context.CommandContext; +import org.ovirt.engine.core.bll.context.CompensationContext; import org.ovirt.engine.core.bll.tasks.CommandCoordinatorUtil; import org.ovirt.engine.core.common.FeatureSupported; import org.ovirt.engine.core.common.action.CreateOvfStoresForStorageDomainCommandParameters; @@ -47,13 +48,11 @@ super(commandId); } + @Override protected void executeCommand() { - StoragePoolIsoMap map = - getStoragePoolIsoMapDAO().get - (new StoragePoolIsoMapId(getParameters().getStorageDomainId(), - getParameters().getStoragePoolId())); - changeDomainStatusWithCompensation(map, StorageDomainStatus.Unknown, StorageDomainStatus.Locked); + StoragePoolIsoMap map = loadStoragePoolIsoMap(); + changeDomainStatusWithCompensation(map, StorageDomainStatus.Unknown, StorageDomainStatus.Locked, getCompensationContext()); if (shouldPerformOvfUpdate()) { runInternalAction(VdcActionType.ProcessOvfUpdateForStoragePool, new StoragePoolParametersBase(getStoragePoolId())); @@ -87,6 +86,12 @@ params.setParentParameters(getParameters()); params.setSkipDomainChecks(true); return params; + } + + private StoragePoolIsoMap loadStoragePoolIsoMap() { + return getStoragePoolIsoMapDAO().get + (new StoragePoolIsoMapId(getParameters().getStorageDomainId(), + getParameters().getStoragePoolId())); } @Override @@ -153,8 +158,13 @@ @Override public void run() { try { + StoragePoolIsoMap map = loadStoragePoolIsoMap(); + CompensationContext context = createDefaultCompensationContext(Guid.newGuid()); + changeDomainStatusWithCompensation(map, StorageDomainStatus.Unknown, StorageDomainStatus.Locked, context); waitForTasksToBeCleared(); executeDeactivateCommnad(false); + context.resetCompensation(); + } catch (Exception e) { setSucceeded(false); log.error("Error when attempting to deactivate storage domain {0}", getStorageDomainId(), e); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java index 29357ea..37392f9 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java @@ -402,24 +402,29 @@ } protected void changeStorageDomainStatusInTransaction(final StoragePoolIsoMap map, - final StorageDomainStatus status) { + final StorageDomainStatus status) { + changeStorageDomainStatusInTransaction(map, status, getCompensationContext()); + } + + protected void changeStorageDomainStatusInTransaction(final StoragePoolIsoMap map, + final StorageDomainStatus status, final CompensationContext context) { executeInNewTransaction(new TransactionMethod<StoragePoolIsoMap>() { @SuppressWarnings("synthetic-access") @Override public StoragePoolIsoMap runInTransaction() { - CompensationContext context = getCompensationContext(); context.snapshotEntityStatus(map); map.setStatus(status); getStoragePoolIsoMapDAO().updateStatus(map.getId(), map.getStatus()); - getCompensationContext().stateChanged(); + context.stateChanged(); return null; } }); } - protected void changeDomainStatusWithCompensation(StoragePoolIsoMap map, StorageDomainStatus compensateStatus, StorageDomainStatus newStatus) { + protected void changeDomainStatusWithCompensation(StoragePoolIsoMap map, StorageDomainStatus compensateStatus, + StorageDomainStatus newStatus, CompensationContext context) { map.setStatus(compensateStatus); - changeStorageDomainStatusInTransaction(map, newStatus); + changeStorageDomainStatusInTransaction(map, newStatus, context); } private StorageDomainStatus getStorageDomainStatus() { -- To view, visit http://gerrit.ovirt.org/35215 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: If4e482a2a13741e8b4509851d9d8e4645d8b81e6 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liron Aravot <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
