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

Reply via email to