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 <[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/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 <[email protected]>
Gerrit-Reviewer: Ravi Nori <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
