Hello Yair Zaslavsky, I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/15570 to review the following change. Change subject: core: Further cleanup for for end of command coordination ...................................................................... core: Further cleanup for for end of command coordination The following patch introduces more cleanup to the code - Renaming the classes to proper names (based on "Command") Changing variables, method names, and fields Change-Id: Ibc3c628b58510e500b24065f5d20253d5d7fba82 Signed-off-by: Yair Zaslavsky <yzasl...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskFactory.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskManager.java R backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandAsyncTask.java R backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandMultiAsyncTasks.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/BackwardCompatibilityTaskCreationTest.java 5 files changed, 50 insertions(+), 50 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/70/15570/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskFactory.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskFactory.java index 3393328..c472fe2 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskFactory.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskFactory.java @@ -67,7 +67,7 @@ if (taskType == AsyncTaskType.unknown) { result = new SPMAsyncTask(asyncTaskParams); } else { - result = new EntityAsyncTask(asyncTaskParams, duringInit); + result = new CommandAsyncTask(asyncTaskParams, duringInit); } return result; } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskManager.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskManager.java index eb1f839..75a90e7 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskManager.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskManager.java @@ -237,7 +237,7 @@ } private boolean isCurrentTaskLookedFor(Guid id, SPMAsyncTask task) { - return (task instanceof EntityAsyncTask) && id.equals(task.getParameters().getEntityInfo().getId()) + return (task instanceof CommandAsyncTask) && id.equals(task.getParameters().getEntityInfo().getId()) && (task.getState() != AsyncTaskState.Cleared) && (task.getState() != AsyncTaskState.ClearFailed); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/EntityAsyncTask.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandAsyncTask.java similarity index 76% rename from backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/EntityAsyncTask.java rename to backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandAsyncTask.java index 72c6935..2a9cf2e 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/EntityAsyncTask.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandAsyncTask.java @@ -23,28 +23,28 @@ /** * Base class for all tasks regarding a specific command. */ -public class EntityAsyncTask extends SPMAsyncTask { +public class CommandAsyncTask extends SPMAsyncTask { private static final Object _lockObject = new Object(); - private static final Map<Guid, EntityMultiAsyncTasks> _multiTasksByCommandIds = - new HashMap<Guid, EntityMultiAsyncTasks>(); + private static final Map<Guid, CommandMultiAsyncTasks> _multiTasksByCommandIds = + new HashMap<Guid, CommandMultiAsyncTasks>(); - private EntityMultiAsyncTasks GetEntityMultiAsyncTasks() { - EntityMultiAsyncTasks entityInfo = null; + private CommandMultiAsyncTasks GetCommandMultiAsyncTasks() { + CommandMultiAsyncTasks entityInfo = null; synchronized (_lockObject) { entityInfo = _multiTasksByCommandIds.get(getCommandId()); } return entityInfo; } - public EntityAsyncTask(AsyncTaskParameters parameters, boolean duringInit) { + public CommandAsyncTask(AsyncTaskParameters parameters, boolean duringInit) { super(parameters); boolean isNewCommandAdded = false; synchronized (_lockObject) { if (!_multiTasksByCommandIds.containsKey(getCommandId())) { - log.infoFormat("EntityAsyncTask::Adding EntityMultiAsyncTasks object for command '{0}'", + log.infoFormat("CommandAsyncTask::Adding CommandMultiAsyncTasks object for command '{0}'", getCommandId()); - _multiTasksByCommandIds.put(getCommandId(), new EntityMultiAsyncTasks(getCommandId())); + _multiTasksByCommandIds.put(getCommandId(), new CommandMultiAsyncTasks(getCommandId())); isNewCommandAdded = true; } } @@ -59,13 +59,13 @@ } } - EntityMultiAsyncTasks entityInfo = GetEntityMultiAsyncTasks(); + CommandMultiAsyncTasks entityInfo = GetCommandMultiAsyncTasks(); entityInfo.AttachTask(this); } @Override protected void ConcreteStartPollingTask() { - EntityMultiAsyncTasks entityInfo = GetEntityMultiAsyncTasks(); + CommandMultiAsyncTasks entityInfo = GetCommandMultiAsyncTasks(); entityInfo.StartPollingTask(getVdsmTaskId()); } @@ -76,10 +76,10 @@ } private void EndActionIfNecessary() { - EntityMultiAsyncTasks entityInfo = GetEntityMultiAsyncTasks(); + CommandMultiAsyncTasks entityInfo = GetCommandMultiAsyncTasks(); if (entityInfo == null) { log.warnFormat( - "EntityAsyncTask::EndActionIfNecessary: No info is available for entity '{0}', current task ('{1}') was probably created while other tasks were in progress, clearing task.", + "CommandAsyncTask::EndActionIfNecessary: No info is available for entity '{0}', current task ('{1}') was probably created while other tasks were in progress, clearing task.", getCommandId(), getVdsmTaskId()); @@ -88,11 +88,11 @@ else if (entityInfo.ShouldEndAction()) { log.infoFormat( - "EntityAsyncTask::EndActionIfNecessary: All tasks of entity '{0}' has ended -> executing 'EndAction'", + "CommandAsyncTask::EndActionIfNecessary: All tasks of entity '{0}' has ended -> executing 'EndAction'", getCommandId()); log.infoFormat( - "EntityAsyncTask::EndAction: Ending action for {0} tasks (command ID: '{1}'): calling EndAction '.", + "CommandAsyncTask::EndAction: Ending action for {0} tasks (command ID: '{1}'): calling EndAction '.", entityInfo.getTasksCountCurrentActionType(), entityInfo.getCommandId()); @@ -108,7 +108,7 @@ } private void EndCommandAction() { - EntityMultiAsyncTasks entityInfo = GetEntityMultiAsyncTasks(); + CommandMultiAsyncTasks entityInfo = GetCommandMultiAsyncTasks(); VdcReturnValueBase vdcReturnValue = null; ExecutionContext context = null; @@ -128,7 +128,7 @@ dbAsyncTask.getActionParameters().setImagesParameters(imagesParameters); try { - log.infoFormat("EntityAsyncTask::EndCommandAction [within thread] context: Attempting to EndAction '{0}', executionIndex: '{1}'", + log.infoFormat("CommandAsyncTask::EndCommandAction [within thread] context: Attempting to EndAction '{0}', executionIndex: '{1}'", dbAsyncTask.getActionParameters().getCommandType(), dbAsyncTask.getActionParameters().getExecutionIndex()); @@ -156,7 +156,7 @@ catch (RuntimeException Ex2) { log.error( - "EntityAsyncTask::EndCommandAction [within thread]: An exception has been thrown (not related to 'EndAction' itself)", + "CommandAsyncTask::EndCommandAction [within thread]: An exception has been thrown (not related to 'EndAction' itself)", Ex2); } @@ -179,18 +179,18 @@ getParameters().getDbAsyncTask().getActionParameters().getCommandType()); } - private void handleEndActionResult(EntityMultiAsyncTasks commandInfo, VdcReturnValueBase vdcReturnValue, + private void handleEndActionResult(CommandMultiAsyncTasks commandInfo, VdcReturnValueBase vdcReturnValue, ExecutionContext context, boolean isTaskGroupSuccess) { try { VdcActionType actionType = getParameters().getDbAsyncTask().getaction_type(); log.infoFormat( - "EntityAsyncTask::HandleEndActionResult [within thread]: EndAction for action type '{0}' completed, handling the result.", + "CommandAsyncTask::HandleEndActionResult [within thread]: EndAction for action type '{0}' completed, handling the result.", actionType); if (vdcReturnValue == null || (!vdcReturnValue.getSucceeded() && vdcReturnValue.getEndActionTryAgain())) { log.infoFormat( - "EntityAsyncTask::HandleEndActionResult [within thread]: EndAction for action type {0} hasn't succeeded, not clearing tasks, will attempt again next polling.", + "CommandAsyncTask::HandleEndActionResult [within thread]: EndAction for action type {0} hasn't succeeded, not clearing tasks, will attempt again next polling.", actionType); commandInfo.Repoll(); @@ -198,7 +198,7 @@ else { log.infoFormat( - "EntityAsyncTask::HandleEndActionResult [within thread]: EndAction for action type {0} {1}succeeded, clearing tasks.", + "CommandAsyncTask::HandleEndActionResult [within thread]: EndAction for action type {0} {1}succeeded, clearing tasks.", actionType, (vdcReturnValue.getSucceeded() ? "" : "hasn't ")); @@ -215,7 +215,7 @@ synchronized (_lockObject) { if (commandInfo.getAllCleared()) { log.infoFormat( - "EntityAsyncTask::HandleEndActionResult [within thread]: Removing EntityMultiAsyncTasks object for entity '{0}'", + "CommandAsyncTask::HandleEndActionResult [within thread]: Removing CommandMultiAsyncTasks object for entity '{0}'", commandInfo.getCommandId()); _multiTasksByCommandIds.remove(commandInfo.getCommandId()); } @@ -224,7 +224,7 @@ } catch (RuntimeException ex) { - log.error("EntityAsyncTask::HandleEndActionResult [within thread]: an exception has been thrown", ex); + log.error("CommandAsyncTask::HandleEndActionResult [within thread]: an exception has been thrown", ex); } } @@ -241,5 +241,5 @@ } - private static final Log log = LogFactory.getLog(EntityAsyncTask.class); + private static final Log log = LogFactory.getLog(CommandAsyncTask.class); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/EntityMultiAsyncTasks.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandMultiAsyncTasks.java similarity index 71% rename from backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/EntityMultiAsyncTasks.java rename to backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandMultiAsyncTasks.java index 91ba9c2..92fa92d 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/EntityMultiAsyncTasks.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandMultiAsyncTasks.java @@ -6,9 +6,9 @@ import org.ovirt.engine.core.utils.log.Log; import org.ovirt.engine.core.utils.log.LogFactory; -public class EntityMultiAsyncTasks { +public class CommandMultiAsyncTasks { - private java.util.HashMap<Guid, EntityAsyncTask> _listTasks; + private java.util.HashMap<Guid, CommandAsyncTask> _listTasks; private Guid commandId; public Guid getCommandId() { @@ -19,12 +19,12 @@ commandId = value; } - public EntityMultiAsyncTasks(Guid commandId) { - _listTasks = new java.util.HashMap<Guid, EntityAsyncTask>(); + public CommandMultiAsyncTasks(Guid commandId) { + _listTasks = new java.util.HashMap<Guid, CommandAsyncTask>(); setCommandId(commandId); } - public void AttachTask(EntityAsyncTask asyncTask) { + public void AttachTask(CommandAsyncTask asyncTask) { synchronized (_listTasks) { if (!_listTasks.containsKey(asyncTask.getVdsmTaskId())) { log.infoFormat("EntityMultiAsyncTasks::AttachTask: Attaching task '{0}' to command '{1}'.", @@ -35,10 +35,10 @@ } } - private java.util.ArrayList<EntityAsyncTask> getCurrentTasks() { - java.util.ArrayList<EntityAsyncTask> retValue = new java.util.ArrayList<EntityAsyncTask>(); + private java.util.ArrayList<CommandAsyncTask> getCurrentTasks() { + java.util.ArrayList<CommandAsyncTask> retValue = new java.util.ArrayList<CommandAsyncTask>(); - for (EntityAsyncTask task : _listTasks.values()) { + for (CommandAsyncTask task : _listTasks.values()) { if (task.getParameters() != null && task.getParameters().getDbAsyncTask() != null && (task.getState() == AsyncTaskState.Polling || task.getState() == AsyncTaskState.Ended || task @@ -52,9 +52,9 @@ public boolean ShouldEndAction() { synchronized (_listTasks) { - java.util.ArrayList<EntityAsyncTask> CurrentActionTypeTasks = getCurrentTasks(); + java.util.ArrayList<CommandAsyncTask> CurrentActionTypeTasks = getCurrentTasks(); - for (EntityAsyncTask task : CurrentActionTypeTasks) { + for (CommandAsyncTask task : CurrentActionTypeTasks) { if (task.getState() != AsyncTaskState.Ended) { return false; } @@ -66,9 +66,9 @@ public void MarkAllWithAttemptingEndAction() { synchronized (_listTasks) { - java.util.ArrayList<EntityAsyncTask> CurrentActionTypeTasks = getCurrentTasks(); + java.util.ArrayList<CommandAsyncTask> CurrentActionTypeTasks = getCurrentTasks(); - for (EntityAsyncTask task : CurrentActionTypeTasks) { + for (CommandAsyncTask task : CurrentActionTypeTasks) { task.setState(AsyncTaskState.AttemptingEndAction); } } @@ -79,9 +79,9 @@ java.util.ArrayList<EndedTaskInfo> endedTaskInfoList = new java.util.ArrayList<EndedTaskInfo>(); synchronized (_listTasks) { - java.util.ArrayList<EntityAsyncTask> CurrentActionTypeTasks = getCurrentTasks(); + java.util.ArrayList<CommandAsyncTask> CurrentActionTypeTasks = getCurrentTasks(); - for (EntityAsyncTask task : CurrentActionTypeTasks) { + for (CommandAsyncTask task : CurrentActionTypeTasks) { task.setLastStatusAccessTime(); EndedTaskInfo tempVar = new EndedTaskInfo(); tempVar.setTaskStatus(task.getLastTaskStatus()); @@ -98,7 +98,7 @@ public int getTasksCountCurrentActionType() { int returnValue = 0; synchronized (_listTasks) { - java.util.ArrayList<EntityAsyncTask> CurrentActionTypeTasks = getCurrentTasks(); + java.util.ArrayList<CommandAsyncTask> CurrentActionTypeTasks = getCurrentTasks(); returnValue = CurrentActionTypeTasks.size(); } @@ -107,9 +107,9 @@ public void Repoll() { synchronized (_listTasks) { - java.util.ArrayList<EntityAsyncTask> CurrentActionTypeTasks = getCurrentTasks(); + java.util.ArrayList<CommandAsyncTask> CurrentActionTypeTasks = getCurrentTasks(); - for (EntityAsyncTask task : CurrentActionTypeTasks) { + for (CommandAsyncTask task : CurrentActionTypeTasks) { task.setState(AsyncTaskState.Ended); } } @@ -117,9 +117,9 @@ public void ClearTasks() { synchronized (_listTasks) { - java.util.ArrayList<EntityAsyncTask> CurrentActionTypeTasks = getCurrentTasks(); + java.util.ArrayList<CommandAsyncTask> CurrentActionTypeTasks = getCurrentTasks(); - for (EntityAsyncTask task : CurrentActionTypeTasks) { + for (CommandAsyncTask task : CurrentActionTypeTasks) { task.clearAsyncTask(); } } @@ -129,7 +129,7 @@ synchronized (_listTasks) { if (_listTasks.containsKey(TaskID) && _listTasks.get(TaskID).getParameters() != null && _listTasks.get(TaskID).getParameters().getDbAsyncTask() != null) { - for (EntityAsyncTask task : _listTasks.values()) { + for (CommandAsyncTask task : _listTasks.values()) { if (task.getParameters() != null && task.getParameters().getDbAsyncTask() != null) task.setState(AsyncTaskState.Polling); } @@ -144,7 +144,7 @@ public boolean getAllCleared() { synchronized (_listTasks) { - for (EntityAsyncTask task : _listTasks.values()) { + for (CommandAsyncTask task : _listTasks.values()) { if (!taskWasCleared(task)) { return false; } @@ -159,10 +159,10 @@ * The task to check. * @return Whether the task is cleared (succeeded or failed) or not cleared. */ - private boolean taskWasCleared(EntityAsyncTask task) { + private boolean taskWasCleared(CommandAsyncTask task) { AsyncTaskState taskState = task.getState(); return taskState == AsyncTaskState.Cleared || taskState == AsyncTaskState.ClearFailed; } - private static Log log = LogFactory.getLog(EntityMultiAsyncTasks.class); + private static Log log = LogFactory.getLog(CommandMultiAsyncTasks.class); } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/BackwardCompatibilityTaskCreationTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/BackwardCompatibilityTaskCreationTest.java index 501d52c..2cc6751 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/BackwardCompatibilityTaskCreationTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/BackwardCompatibilityTaskCreationTest.java @@ -163,7 +163,7 @@ assertEquals("wrong task result", AsyncTaskResultEnum.success, spmAsyncTask.getLastTaskStatus().getResult()); assertEquals("wrong task status", AsyncTaskStatusEnum.init, spmAsyncTask.getLastTaskStatus().getStatus()); assertEquals("wrong task state", AsyncTaskState.Initializing, spmAsyncTask.getState()); - assertTrue("wrong task type", spmAsyncTask instanceof EntityAsyncTask); + assertTrue("wrong task type", spmAsyncTask instanceof CommandAsyncTask); } /** -- To view, visit http://gerrit.ovirt.org/15570 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ibc3c628b58510e500b24065f5d20253d5d7fba82 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches