Hello Ravi Nori, I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/29830 to review the following change. Change subject: engine : Save CommandContext in ContextCache ...................................................................... engine : Save CommandContext in ContextCache Cache the command context in ContextCache which is backed by a map. Change-Id: I6b968f787b60f83cf96411a97dd6380421f724b9 Bug-Url: https://bugzilla.redhat.com/1115127 Signed-off-by: Ravi Nori <rn...@redhat.com> --- 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/DestroyImageCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MergeCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MergeCommandCallback.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MergeStatusCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotSingleDiskLiveCommand.java A backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandContextsCacheImpl.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandCoordinatorImpl.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandExecutor.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/TaskManagerUtil.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/interfaces/CommandCRUDOperations.java A backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/interfaces/CommandContextsCache.java 13 files changed, 89 insertions(+), 21 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/30/29830/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 cdd3f42..5f8b4a2 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 @@ -2105,11 +2105,15 @@ getReturnValue().setCanDoAction(internalReturnValue.getCanDoAction()); } - public void persistCommand(VdcActionType parentCommand) { - persistCommand(parentCommand, false); + public void persistCommandWithoutContext(VdcActionType parentCommand, boolean enableCallBack) { + persistCommand(parentCommand, null, enableCallBack); } - public void persistCommand(VdcActionType parentCommand, boolean enableCallBack) { + public void persistCommandWithContext(VdcActionType parentCommand, boolean enableCallBack) { + persistCommand(parentCommand, getContext(), enableCallBack); + } + + public void persistCommand(VdcActionType parentCommand, CommandContext cmdContext, boolean enableCallBack) { VdcActionParametersBase parentParameters = getParentParameters(parentCommand); Transaction transaction = TransactionSupport.suspend(); try { @@ -2120,7 +2124,8 @@ getParameters(), commandStatus, enableCallBack, - getReturnValue())); + getReturnValue()), + cmdContext); } finally { TransactionSupport.resume(transaction); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/DestroyImageCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/DestroyImageCommand.java index fa5a174..50ce643 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/DestroyImageCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/DestroyImageCommand.java @@ -49,7 +49,7 @@ getReturnValue().getVdsmTaskIdList().add(result); getParameters().getVdsmTaskIds().add(result); setSucceeded(vdsReturnValue.getSucceeded()); - persistCommand(getParameters().getParentCommand(), true); + persistCommandWithContext(getParameters().getParentCommand(), true); log.info("Successfully started task to remove orphaned volumes resulting from live merge"); } else { setSucceeded(false); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MergeCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MergeCommand.java index 9bf96bf..66aed2e 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MergeCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MergeCommand.java @@ -46,7 +46,7 @@ getParameters().setVmJobId(jobId); // setSucceeded to indicate executeCommand success; doPolling will check commandStatus setSucceeded(true); - persistCommand(getParameters().getParentCommand(), true); + persistCommandWithContext(getParameters().getParentCommand(), true); log.debug("Merge started successfully"); } else { log.error("Failed to start Merge on VDS"); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MergeCommandCallback.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MergeCommandCallback.java index 09e5635..4ee03b1 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MergeCommandCallback.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MergeCommandCallback.java @@ -35,7 +35,7 @@ // It finished; a command will be called later to determine the status. command.setSucceeded(true); command.setCommandStatus(CommandStatus.SUCCEEDED); - command.persistCommand(command.getParameters().getParentCommand(), true); + command.persistCommandWithContext(command.getParameters().getParentCommand(), true); } log.infoFormat("Merge command has completed for images {0}..{1}", command.getParameters().getBaseImage().getImageId(), diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MergeStatusCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MergeStatusCommand.java index 03c3e0e..b1c47e5 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MergeStatusCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MergeStatusCommand.java @@ -75,7 +75,7 @@ MergeStatusReturnValue returnValue = new MergeStatusReturnValue(jobType, imagesToRemove); getReturnValue().setActionReturnValue(returnValue); setSucceeded(true); - persistCommand(getParameters().getParentCommand(), true); + persistCommandWithContext(getParameters().getParentCommand(), true); setCommandStatus(CommandStatus.SUCCEEDED); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java index 0baafee..d925752 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java @@ -131,7 +131,7 @@ if (getSnapshotActionType() == VdcActionType.RemoveSnapshotSingleDiskLive) { // Enable callbacks in order to monitor for new-style child completion setCommandStatus(CommandStatus.ACTIVE_ASYNC); - persistCommand(getParameters().getParentCommand(), true); + persistCommandWithoutContext(getParameters().getParentCommand(), true); } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotSingleDiskLiveCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotSingleDiskLiveCommand.java index 64110ef..ace0b5a 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotSingleDiskLiveCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotSingleDiskLiveCommand.java @@ -140,7 +140,7 @@ break; } - persistCommand(getParameters().getParentCommand(), true); + persistCommandWithContext(getParameters().getParentCommand(), true); if (nextCommand != null) { TaskManagerUtil.executeAsyncCommand(nextCommand.getFirst(), nextCommand.getSecond(), cloneContextAndDetachFromParent()); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandContextsCacheImpl.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandContextsCacheImpl.java new file mode 100644 index 0000000..13c5f94 --- /dev/null +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandContextsCacheImpl.java @@ -0,0 +1,31 @@ +package org.ovirt.engine.core.bll.tasks; + +import org.ovirt.engine.core.bll.context.CommandContext; +import org.ovirt.engine.core.bll.tasks.interfaces.CommandContextsCache; +import org.ovirt.engine.core.compat.Guid; + +public class CommandContextsCacheImpl implements CommandContextsCache { + + private static final String COMMAND_CONTEXT_MAP_NAME = "commandContextMap"; + CacheWrapper<Guid, CommandContext> contextsMap; + + public CommandContextsCacheImpl() { + contextsMap = CacheProviderFactory.<Guid, CommandContext> getCacheWrapper(COMMAND_CONTEXT_MAP_NAME); + } + + @Override + public CommandContext get(Guid commandId) { + return contextsMap.get(commandId); + } + + @Override + public void remove(final Guid commandId) { + contextsMap.remove(commandId); + } + + @Override + public void put(final Guid cmdId, final CommandContext context) { + contextsMap.put(cmdId, context); + } + +} diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandCoordinatorImpl.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandCoordinatorImpl.java index a71e5fc..247636e 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandCoordinatorImpl.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandCoordinatorImpl.java @@ -13,6 +13,7 @@ import org.ovirt.engine.core.bll.context.CommandContext; import org.ovirt.engine.core.bll.interfaces.BackendInternal; import org.ovirt.engine.core.bll.job.ExecutionContext; +import org.ovirt.engine.core.bll.tasks.interfaces.CommandContextsCache; import org.ovirt.engine.core.bll.tasks.interfaces.CommandCoordinator; import org.ovirt.engine.core.bll.tasks.interfaces.SPMTask; import org.ovirt.engine.core.common.VdcObjectType; @@ -40,6 +41,7 @@ private static final Log log = LogFactory.getLog(CommandCoordinator.class); private final CommandsCache commandsCache; + private final CommandContextsCache contextsCache; private final CoCoAsyncTaskHelper coCoAsyncTaskHelper; private final CommandExecutor cmdExecutor; private Object LOCK = new Object(); @@ -48,12 +50,19 @@ CommandCoordinatorImpl() { commandsCache = new CommandsCacheImpl(); + contextsCache = new CommandContextsCacheImpl(); coCoAsyncTaskHelper = new CoCoAsyncTaskHelper(this); cmdExecutor = new CommandExecutor(this); } public <P extends VdcActionParametersBase> CommandBase<P> createCommand(VdcActionType action, P parameters) { return CommandsFactory.createCommand(action, parameters); + } + + @Override + public void persistCommand(CommandEntity cmdEntity, CommandContext cmdContext) { + persistCommand(cmdEntity); + saveCommandContext(cmdEntity.getId(), cmdContext); } @Override @@ -64,6 +73,12 @@ if (!cmdEntity.isCallBackNotified()) { cmdExecutor.addToCallBackMap(cmdEntity); } + } + } + + void saveCommandContext(Guid cmdId, CommandContext cmdContext) { + if (cmdContext != null) { + contextsCache.put(cmdId, cmdContext); } } @@ -97,13 +112,13 @@ @Override public CommandBase<?> retrieveCommand(Guid commandId) { - return buildCommand(commandsCache.get(commandId)); + return buildCommand(commandsCache.get(commandId), contextsCache.get(commandId)); } - private CommandBase<?> buildCommand(CommandEntity cmdEntity) { + private CommandBase<?> buildCommand(CommandEntity cmdEntity, CommandContext cmdContext) { CommandBase<?> command = null; if (cmdEntity != null) { - command = CommandsFactory.createCommand(cmdEntity.getCommandType(), cmdEntity.getActionParameters()); + command = CommandsFactory.createCommand(cmdEntity.getCommandType(), cmdEntity.getActionParameters(), cmdContext); command.setCommandStatus(cmdEntity.getCommandStatus(), false); if (!Guid.isNullOrEmpty(cmdEntity.getRootCommandId()) && ! cmdEntity.getRootCommandId().equals(cmdEntity.getId()) && @@ -134,6 +149,7 @@ public void removeCommand(final Guid commandId) { commandsCache.remove(commandId); + contextsCache.remove(commandId); updateCmdHierarchy(commandId); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandExecutor.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandExecutor.java index ed08b5e..80692d2 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandExecutor.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandExecutor.java @@ -14,7 +14,6 @@ import org.ovirt.engine.core.bll.CommandsFactory; import org.ovirt.engine.core.bll.context.CommandContext; import org.ovirt.engine.core.bll.tasks.interfaces.CommandCallBack; -import org.ovirt.engine.core.bll.tasks.interfaces.CommandCoordinator; import org.ovirt.engine.core.bll.utils.BackendUtils; import org.ovirt.engine.core.common.action.VdcActionParametersBase; import org.ovirt.engine.core.common.action.VdcActionType; @@ -35,11 +34,11 @@ private static final ExecutorService executor = Executors.newFixedThreadPool(Config.<Integer>getValue(ConfigValues.CommandCoordinatorThreadPoolSize)); private static final Log log = LogFactory.getLog(CommandExecutor.class); - private final CommandCoordinator coco; + private final CommandCoordinatorImpl coco; private final Map<Guid, CommandCallBack> cmdCallBackMap = new ConcurrentHashMap<>(); private boolean cmdExecutorInitialized; - CommandExecutor(CommandCoordinator coco) { + CommandExecutor(CommandCoordinatorImpl coco) { this.coco = coco; SchedulerUtil scheduler = SchedulerUtilQuartzImpl.getInstance(); scheduler.scheduleAFixedDelayJob(this, "invokeCallbackMethods", new Class[]{}, @@ -104,17 +103,18 @@ final VdcActionParametersBase parameters, final CommandContext cmdContext) { final CommandBase<?> command = CommandsFactory.createCommand(actionType, parameters, cmdContext); + coco.saveCommandContext(command.getCommandId(), cmdContext); return executor.submit(new Callable<VdcReturnValueBase>() { @Override public VdcReturnValueBase call() throws Exception { - return executeCommand(command); + return executeCommand(command, cmdContext); } }); } - private VdcReturnValueBase executeCommand(final CommandBase<?> command) { - command.persistCommand(command.getParameters().getParentCommand(), true); + private VdcReturnValueBase executeCommand(final CommandBase<?> command, final CommandContext cmdContext) { + command.persistCommand(command.getParameters().getParentCommand(), cmdContext, true); CommandCallBack callBack = command.getCallBack(); if (callBack != null) { cmdCallBackMap.put(command.getCommandId(), callBack); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/TaskManagerUtil.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/TaskManagerUtil.java index e3ef9d3..44e8d17 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/TaskManagerUtil.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/TaskManagerUtil.java @@ -131,8 +131,8 @@ coco.addOrUpdateTaskInDB(asyncTask); } - public static void persistCommand(CommandEntity cmdEntity) { - coco.persistCommand(cmdEntity); + public static void persistCommand(CommandEntity cmdEntity, CommandContext cmdContext) { + coco.persistCommand(cmdEntity, cmdContext); } public static List<Guid> getChildCommandIds(Guid commandId) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/interfaces/CommandCRUDOperations.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/interfaces/CommandCRUDOperations.java index ad71e01..de8da30 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/interfaces/CommandCRUDOperations.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/interfaces/CommandCRUDOperations.java @@ -1,6 +1,7 @@ package org.ovirt.engine.core.bll.tasks.interfaces; import org.ovirt.engine.core.bll.CommandBase; +import org.ovirt.engine.core.bll.context.CommandContext; import org.ovirt.engine.core.common.asynctasks.AsyncTaskType; import org.ovirt.engine.core.common.businessentities.CommandEntity; import org.ovirt.engine.core.compat.CommandStatus; @@ -15,6 +16,7 @@ public CommandStatus getCommandStatus(Guid commandId); public List<CommandEntity> getCommandsWithCallBackEnabled(); public void persistCommand(CommandEntity cmdEntity); + public void persistCommand(CommandEntity cmdEntity, CommandContext cmdContext); public CommandBase<?> retrieveCommand(Guid commandId); public void removeCommand(Guid commandId); public void removeAllCommandsInHierarchy(Guid commandId); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/interfaces/CommandContextsCache.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/interfaces/CommandContextsCache.java new file mode 100644 index 0000000..d64d809 --- /dev/null +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/interfaces/CommandContextsCache.java @@ -0,0 +1,14 @@ +package org.ovirt.engine.core.bll.tasks.interfaces; + +import org.ovirt.engine.core.bll.context.CommandContext; +import org.ovirt.engine.core.compat.Guid; + +public interface CommandContextsCache { + + public CommandContext get(Guid commandId); + + public void remove(Guid commandId); + + public void put(Guid commandId, CommandContext context); + +} -- To view, visit http://gerrit.ovirt.org/29830 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6b968f787b60f83cf96411a97dd6380421f724b9 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.5 Gerrit-Owner: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Ravi Nori <rn...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches