Ravi Nori has uploaded a new change for review. Change subject: engine : CommandExecutor should handle exception in CallBackMethods ......................................................................
engine : CommandExecutor should handle exception in CallBackMethods If one of the callback methods raises an exception, the framework keeps calling the method again the next loop iteration. In some circumstances this means the command will never converge--e.g. coding errors, database connection failures, etc. Change-Id: I06b82f29ac31fd09903d76f87e839d8ac32ef1e1 Bug-Url: https://bugzilla.redhat.com/1121237 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/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/CommandsCache.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandsCacheImpl.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 7 files changed, 57 insertions(+), 32 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/03/30703/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 8610966..a9308c4 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 @@ -2153,7 +2153,7 @@ if (updateDB) { Transaction transaction = TransactionSupport.suspend(); try { - TaskManagerUtil.updateCommandStatus(getCommandId(), getTaskType(), commandStatus); + TaskManagerUtil.updateCommandStatus(getCommandId(), commandStatus); } finally { if (transaction != null) { TransactionSupport.resume(transaction); 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 0227183..8f99f35 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 @@ -174,8 +174,8 @@ } } - public void updateCommandStatus(final Guid commandId, final AsyncTaskType taskType, final CommandStatus status) { - commandsCache.updateCommandStatus(commandId, taskType, status); + public void updateCommandStatus(final Guid commandId, final CommandStatus status) { + commandsCache.updateCommandStatus(commandId, status); } public void updateCommandExecuted(Guid 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 7c60ee1..7322c8b 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 @@ -60,28 +60,59 @@ Guid cmdId = entry.getKey(); CommandCallBack callBack = entry.getValue(); CommandStatus status = coco.getCommandStatus(cmdId); - switch (status) { - case FAILED: - callBack.onFailed(cmdId, coco.getChildCommandIds(cmdId)); - coco.updateCallBackNotified(cmdId); - iterator.remove(); - break; - case SUCCEEDED: - callBack.onSucceeded(cmdId, coco.getChildCommandIds(cmdId)); - coco.updateCallBackNotified(cmdId); - iterator.remove(); - break; - case ACTIVE: - if (coco.getCommandEntity(cmdId).isExecuted()) { - callBack.doPolling(cmdId, coco.getChildCommandIds(cmdId)); + boolean errorInCallback = false; + try { + switch (status) { + case FAILED: + callBack.onFailed(cmdId, coco.getChildCommandIds(cmdId)); + break; + case SUCCEEDED: + callBack.onSucceeded(cmdId, coco.getChildCommandIds(cmdId)); + break; + case ACTIVE: + if (coco.getCommandEntity(cmdId).isExecuted()) { + callBack.doPolling(cmdId, coco.getChildCommandIds(cmdId)); + } + break; + default: + break; } - break; - default: - break; + } catch (Exception ex) { + errorInCallback = true; + handleError(ex, status, cmdId); + } finally { + if (CommandStatus.FAILED.equals(status) || (CommandStatus.SUCCEEDED.equals(status) && !errorInCallback)) { + coco.updateCallBackNotified(cmdId); + iterator.remove(); + } } } } + private void handleError(Exception ex, CommandStatus status, Guid cmdId) { + log.errorFormat("Error invoking callback method {0} for {1} command {2}", + getCallBackMethod(status), + status.toString(), + cmdId.toString()); + log.error(ex); + if (!CommandStatus.FAILED.equals(status)) { + coco.updateCommandStatus(cmdId, CommandStatus.FAILED); + } + } + + private String getCallBackMethod(CommandStatus status) { + switch (status) { + case FAILED: + return "onFailed"; + case SUCCEEDED: + return "onSucceeded"; + case ACTIVE: + return "doPolling"; + default: + return "Unknown"; + } + } + private void initCommandExecutor() { if (!cmdExecutorInitialized) { for (CommandEntity cmdEntity : coco.getCommandsWithCallBackEnabled()) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandsCache.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandsCache.java index cc54a2a..a65421d 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandsCache.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandsCache.java @@ -1,6 +1,5 @@ package org.ovirt.engine.core.bll.tasks; -import org.ovirt.engine.core.common.asynctasks.AsyncTaskType; import org.ovirt.engine.core.common.businessentities.CommandEntity; import org.ovirt.engine.core.compat.CommandStatus; import org.ovirt.engine.core.compat.DateTime; @@ -20,7 +19,7 @@ public void removeAllCommandsBeforeDate(DateTime cutoff); - public void updateCommandStatus(Guid commandId, AsyncTaskType taskType, CommandStatus status); + public void updateCommandStatus(Guid commandId, CommandStatus status); public void updateCommandExecuted(Guid commandId); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandsCacheImpl.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandsCacheImpl.java index 50043bb..c44bbb8 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandsCacheImpl.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandsCacheImpl.java @@ -3,7 +3,6 @@ import java.util.List; import java.util.Set; -import org.ovirt.engine.core.common.asynctasks.AsyncTaskType; import org.ovirt.engine.core.common.businessentities.CommandEntity; import org.ovirt.engine.core.compat.CommandStatus; import org.ovirt.engine.core.compat.DateTime; @@ -64,13 +63,11 @@ initializeCache(); } - public void updateCommandStatus(Guid commandId, AsyncTaskType taskType, CommandStatus status) { + public void updateCommandStatus(Guid commandId, CommandStatus status) { final CommandEntity cmdEntity = get(commandId); if (cmdEntity != null) { cmdEntity.setCommandStatus(status); - if (taskType.equals(AsyncTaskType.notSupported) || cmdEntity.isCallBackEnabled()) { - DbFacade.getInstance().getCommandEntityDao().saveOrUpdate(cmdEntity); - } + DbFacade.getInstance().getCommandEntityDao().saveOrUpdate(cmdEntity); } } 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 bf33989..3022564 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 @@ -13,7 +13,6 @@ import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.action.VdcReturnValueBase; import org.ovirt.engine.core.common.asynctasks.AsyncTaskCreationInfo; -import org.ovirt.engine.core.common.asynctasks.AsyncTaskType; import org.ovirt.engine.core.common.businessentities.AsyncTaskStatus; import org.ovirt.engine.core.common.businessentities.AsyncTasks; import org.ovirt.engine.core.common.businessentities.CommandEntity; @@ -160,8 +159,8 @@ return coco.getCommandStatus(commandId); } - public static void updateCommandStatus(Guid commandId, AsyncTaskType taskType, CommandStatus status) { - coco.updateCommandStatus(commandId, taskType, status); + public static void updateCommandStatus(Guid commandId, CommandStatus status) { + coco.updateCommandStatus(commandId, status); } public static CommandExecutionStatus getCommandExecutionStatus(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 70f98af..f833c2c 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 @@ -2,7 +2,6 @@ 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; import org.ovirt.engine.core.compat.DateTime; @@ -21,7 +20,7 @@ public void removeCommand(Guid commandId); public void removeAllCommandsInHierarchy(Guid commandId); public void removeAllCommandsBeforeDate(DateTime cutoff); - public void updateCommandStatus(Guid commandId, AsyncTaskType taskType, CommandStatus status); + public void updateCommandStatus(Guid commandId, CommandStatus status); public void updateCommandExecuted(Guid commandId); public void updateCallBackNotified(Guid commandId); } -- To view, visit http://gerrit.ovirt.org/30703 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I06b82f29ac31fd09903d76f87e839d8ac32ef1e1 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.5 Gerrit-Owner: Ravi Nori <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
