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 <[email protected]>
---
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 <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
