Arik Hadas has posted comments on this change. Change subject: engine : Command Executor should persist command before submitting to threadpool ......................................................................
Patch Set 2: (2 comments) http://gerrit.ovirt.org/#/c/29219/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandExecutor.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandExecutor.java: Line 94: break; Line 95: case REJECTED: Line 96: case NOT_STARTED: Line 97: executeAsyncCommand(cmdEntity.getCommandType(), cmdEntity.getActionParameters()); Line 98: break; I'm not sure we want to execute every REJECTED/NOT_STARTED command on engine startup, what if it took 2 hours to restart the engine? think of the following case: host went down and we run a VM that ran on it using this infra, the command didn't start and there's an engine is stopped and started after an hour - on startup we'll try to run the VM, even if no SPM was selected, so the user will see that we try to run the VM without asking and that the operation fails I think it is better no to try again to execute the commands in that case. we should do it only for special cases such as HA VMs Line 99: default: Line 100: addToCallBackMap(cmdEntity); Line 101: } Line 102: } Line 116: } Line 117: } Line 118: Line 119: public Future<VdcReturnValueBase> executeAsyncCommand(final VdcActionType actionType, Line 120: final VdcActionParametersBase parameters) { we need to have a way to provide the engine lock in a way that the lock will be taken right after the task is submitted so the entity will be locked until the operation is executed think of the following case: HA VM went down the run operation gets rejected (next try on 10 sec) the user start and right after that stops the VM after that 10 sec, we'll try to run the VM again so I think that on reject we should have: 1. audit log for the user to know that something bad is going on 2. not try to execute the command again or provide a way to pass the context, including the lock, and lock the entity here. the lock should be passed to the command to be executed Line 121: final CommandBase<?> command = CommandsFactory.createCommand(actionType, parameters); Line 122: command.persistCommand(command.getParameters().getParentCommand(), true); Line 123: try { Line 124: return executor.submit(new Callable<VdcReturnValueBase>() { -- To view, visit http://gerrit.ovirt.org/29219 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4d510836dc822a95198dc1db4a4f74206466f0c9 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
