Arik Hadas has uploaded a new change for review.

Change subject: core: filter duplicate requests to run the same vm
......................................................................

core: filter duplicate requests to run the same vm

If engine gets multiple requests to run the same VM at the same time
(with MultipleActionRunner), we might get into a situation where the VM
does not run and kept locked.

Say we got 2 run requests for the same VM:
- The first command acquires the lock for the VM
- The second command fails to acquire the lock
- The can-do-action phase of the first command succeed
- We sort the commands before invoking them. As part of the sort, we map
  Id of VM to its run command. Since we iterate the commands in the same
  order they were received, we end up with mapping the second command
- Since the second command didn't manage to acquire the lock, it won't
  run
Since the first command does not run, it won't release its lock.

The solution is to filter the given parameters from duplicate requests
to run the same VM.

Change-Id: I91320cfd50fc7a12b01afae2885a783c0516a6df
Signed-off-by: Arik Hadas <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MultipleActionsRunner.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVMActionRunner.java
M 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java
3 files changed, 25 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/09/36009/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MultipleActionsRunner.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MultipleActionsRunner.java
index 834c82b..7bffe6d 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MultipleActionsRunner.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MultipleActionsRunner.java
@@ -34,11 +34,19 @@
 
     public MultipleActionsRunner(VdcActionType actionType, 
List<VdcActionParametersBase> parameters, CommandContext commandContext, 
boolean isInternal) {
         this.actionType = actionType;
-        this.parameters = parameters;
+        this.parameters = changeParameters(parameters);
         this.isInternal = isInternal;
         this.commandContext = commandContext;
     }
 
+    /**
+     * Can be overridden by subclasses to modify/filter the given parameters
+     * By default, return the given parameters as-is
+     */
+    protected List<VdcActionParametersBase> 
changeParameters(List<VdcActionParametersBase> parameters) {
+        return parameters;
+    }
+
     protected List<VdcActionParametersBase> getParameters() {
         return parameters;
     }
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVMActionRunner.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVMActionRunner.java
index afdc040..7e92b4e 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVMActionRunner.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVMActionRunner.java
@@ -3,10 +3,12 @@
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 import org.ovirt.engine.core.bll.context.CommandContext;
 import org.ovirt.engine.core.common.action.VdcActionParametersBase;
 import org.ovirt.engine.core.common.action.VdcActionType;
+import org.ovirt.engine.core.common.action.VmOperationParameterBase;
 import org.ovirt.engine.core.compat.Guid;
 import org.ovirt.engine.core.dal.dbbroker.DbFacade;
 
@@ -17,6 +19,16 @@
     }
 
     @Override
+    protected List<VdcActionParametersBase> 
changeParameters(List<VdcActionParametersBase> parameters) {
+        Map<Guid, VdcActionParametersBase> vmIdToParameters = new HashMap<>();
+        for (VdcActionParametersBase parameter : parameters) {
+            VmOperationParameterBase vmParameter = (VmOperationParameterBase) 
parameter;
+            vmIdToParameters.put(vmParameter.getVmId(), vmParameter);
+        }
+        return new 
ArrayList<VdcActionParametersBase>(vmIdToParameters.values());
+    }
+
+    @Override
     protected void sortCommands() {
 
         ArrayList<CommandBase<?>> commandsList = getCommands();
diff --git 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java
 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java
index 15fc3e7..5b6acfc 100644
--- 
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java
+++ 
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java
@@ -1791,6 +1791,10 @@
         {
             VM a = (VM) item;
             list.add(new RunVmParams(a.getId()));
+            list.add(new RunVmParams(a.getId()));
+            list.add(new RunVmParams(a.getId()));
+            list.add(new RunVmParams(a.getId()));
+            list.add(new RunVmParams(a.getId()));
         }
 
         Frontend.getInstance().runMultipleAction(VdcActionType.RunVm, list,


-- 
To view, visit http://gerrit.ovirt.org/36009
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I91320cfd50fc7a12b01afae2885a783c0516a6df
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to