Hello Yair Zaslavsky,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/15569
to review the following change.
Change subject: core: Introducing "end command coordination" by command Id
......................................................................
core: Introducing "end command coordination" by command Id
The following patch changes EntityAsyncTask and EntityMultiAsyncTasks
to perform tasks completion coordination (in order to run endAction) based
on command Ids (the parent command Id), and not the entity Id
Change-Id: I53999748c791aae04287efaa01c10ea7037e5ff2
Signed-off-by: Yair Zaslavsky <[email protected]>
---
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskState.java
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/EntityAsyncTask.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/EntityMultiAsyncTasks.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SPMAsyncTask.java
5 files changed, 69 insertions(+), 159 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/69/15569/1
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskState.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskState.java
index 40e1f3c..e915b95 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskState.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskState.java
@@ -2,7 +2,6 @@
public enum AsyncTaskState {
Initializing,
- WaitForPoll,
Polling,
Ended,
AttemptingEndAction,
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 17a0029..f129e4e 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
@@ -1072,6 +1072,7 @@
// to the one of the parent command (if its REGULAR_FLOW, the execution
// reason of the parent command remains REGULAR_FLOW).
parentParameters.setExecutionReason(parameters.getExecutionReason());
+ parentParameters.setCommandType(parentCommandType);
return parentParameters;
}
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/EntityAsyncTask.java
index 1a70b14..72c6935 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/EntityAsyncTask.java
@@ -8,48 +8,43 @@
import org.ovirt.engine.core.bll.job.ExecutionContext;
import org.ovirt.engine.core.bll.job.ExecutionHandler;
import org.ovirt.engine.core.common.action.VdcActionParametersBase;
+import org.ovirt.engine.core.common.action.VdcActionType;
import org.ovirt.engine.core.common.action.VdcReturnValueBase;
import org.ovirt.engine.core.common.asynctasks.AsyncTaskParameters;
import org.ovirt.engine.core.common.asynctasks.EndedTaskInfo;
import org.ovirt.engine.core.common.businessentities.AsyncTasks;
import org.ovirt.engine.core.common.errors.VdcBLLException;
+import org.ovirt.engine.core.compat.Guid;
import org.ovirt.engine.core.compat.NGuid;
import org.ovirt.engine.core.utils.log.Log;
import org.ovirt.engine.core.utils.log.LogFactory;
import org.ovirt.engine.core.utils.threadpool.ThreadPoolUtil;
/**
- * EntityAsyncTask: Base class for all tasks regarding a specific entity (VM,
- * VmTemplate). The 'OnAfterEntityTaskEnded' method will be executed only if
all
- * other tasks regarding the relevant entity have already ended.
+ * Base class for all tasks regarding a specific command.
*/
public class EntityAsyncTask extends SPMAsyncTask {
private static final Object _lockObject = new Object();
- private static final Map<Object, EntityMultiAsyncTasks>
_multiTasksByEntities =
- new HashMap<Object, EntityMultiAsyncTasks>();
-
- private static EntityMultiAsyncTasks
GetEntityMultiAsyncTasksByContainerId(Object containerID) {
- EntityMultiAsyncTasks entityInfo = null;
- synchronized (_lockObject) {
- entityInfo = _multiTasksByEntities.get(containerID);
- }
-
- return entityInfo;
- }
+ private static final Map<Guid, EntityMultiAsyncTasks>
_multiTasksByCommandIds =
+ new HashMap<Guid, EntityMultiAsyncTasks>();
private EntityMultiAsyncTasks GetEntityMultiAsyncTasks() {
- return GetEntityMultiAsyncTasksByContainerId(getContainerId());
+ EntityMultiAsyncTasks entityInfo = null;
+ synchronized (_lockObject) {
+ entityInfo = _multiTasksByCommandIds.get(getCommandId());
+ }
+ return entityInfo;
}
public EntityAsyncTask(AsyncTaskParameters parameters, boolean duringInit)
{
super(parameters);
boolean isNewCommandAdded = false;
synchronized (_lockObject) {
- if (!_multiTasksByEntities.containsKey(getContainerId())) {
- log.infoFormat("EntityAsyncTask::Adding EntityMultiAsyncTasks
object for entity '{0}'",
- getContainerId());
- _multiTasksByEntities.put(getContainerId(), new
EntityMultiAsyncTasks(getContainerId()));
+ if (!_multiTasksByCommandIds.containsKey(getCommandId())) {
+ log.infoFormat("EntityAsyncTask::Adding EntityMultiAsyncTasks
object for command '{0}'",
+ getCommandId());
+ _multiTasksByCommandIds.put(getCommandId(), new
EntityMultiAsyncTasks(getCommandId()));
isNewCommandAdded = true;
}
}
@@ -85,7 +80,7 @@
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.",
- getContainerId(),
+ getCommandId(),
getVdsmTaskId());
clearAsyncTask();
@@ -94,13 +89,12 @@
else if (entityInfo.ShouldEndAction()) {
log.infoFormat(
"EntityAsyncTask::EndActionIfNecessary: All tasks of
entity '{0}' has ended -> executing 'EndAction'",
- getContainerId());
+ getCommandId());
log.infoFormat(
- "EntityAsyncTask::EndAction: Ending action for {0} tasks
(entity ID: '{1}'): calling EndAction for action type '{2}'.",
+ "EntityAsyncTask::EndAction: Ending action for {0} tasks
(command ID: '{1}'): calling EndAction '.",
entityInfo.getTasksCountCurrentActionType(),
- entityInfo.getContainerId(),
- entityInfo.getActionType());
+ entityInfo.getCommandId());
entityInfo.MarkAllWithAttemptingEndAction();
ThreadPoolUtil.execute(new Runnable() {
@@ -135,7 +129,7 @@
try {
log.infoFormat("EntityAsyncTask::EndCommandAction [within thread]
context: Attempting to EndAction '{0}', executionIndex: '{1}'",
- entityInfo.getActionType(),
+ dbAsyncTask.getActionParameters().getCommandType(),
dbAsyncTask.getActionParameters().getExecutionIndex());
try {
@@ -148,15 +142,15 @@
}
vdcReturnValue =
-
Backend.getInstance().endAction(entityInfo.getActionType(),
+
Backend.getInstance().endAction(getEndActionType(dbAsyncTask),
dbAsyncTask.getActionParameters(),
new CommandContext(context));
} catch (VdcBLLException ex) {
- log.error(getErrorMessage(entityInfo));
+ log.error(getErrorMessage());
log.error(ex.toString());
log.debug(ex);
} catch (RuntimeException ex) {
- log.error(getErrorMessage(entityInfo), ex);
+ log.error(getErrorMessage(), ex);
}
}
@@ -172,33 +166,40 @@
}
}
- private static String getErrorMessage(EntityMultiAsyncTasks entityInfo) {
- return String.format("[within thread]: EndAction for action type %1$s
threw an exception.",
- entityInfo.getActionType());
+ private VdcActionType getEndActionType(AsyncTasks dbAsyncTask) {
+ VdcActionType commandType =
dbAsyncTask.getActionParameters().getCommandType();
+ if (!VdcActionType.Unknown.equals(commandType)) {
+ return commandType;
+ }
+ return dbAsyncTask.getaction_type();
}
- private static void handleEndActionResult(EntityMultiAsyncTasks entityInfo,
- VdcReturnValueBase vdcReturnValue,
+ private String getErrorMessage() {
+ return String.format("[within thread]: EndAction for action type %1$s
threw an exception.",
+
getParameters().getDbAsyncTask().getActionParameters().getCommandType());
+ }
+
+ private void handleEndActionResult(EntityMultiAsyncTasks commandInfo,
VdcReturnValueBase vdcReturnValue,
ExecutionContext context,
boolean isTaskGroupSuccess) {
try {
- if (entityInfo != null) {
- log.infoFormat(
- "EntityAsyncTask::HandleEndActionResult [within
thread]: EndAction for action type '{0}' completed, handling the result.",
- entityInfo.getActionType());
+ VdcActionType actionType =
getParameters().getDbAsyncTask().getaction_type();
+ log.infoFormat(
+ "EntityAsyncTask::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.",
- entityInfo.getActionType());
+ actionType);
- entityInfo.Repoll();
+ commandInfo.Repoll();
}
else {
log.infoFormat(
"EntityAsyncTask::HandleEndActionResult [within
thread]: EndAction for action type {0} {1}succeeded, clearing tasks.",
- entityInfo.getActionType(),
+ actionType,
(vdcReturnValue.getSucceeded() ? "" : "hasn't "));
/**
@@ -209,21 +210,17 @@
ExecutionHandler.endTaskJob(context,
vdcReturnValue.getSucceeded() && isTaskGroupSuccess);
}
- entityInfo.ClearTasks();
+ commandInfo.ClearTasks();
synchronized (_lockObject) {
- if (entityInfo.getAllCleared()) {
+ if (commandInfo.getAllCleared()) {
log.infoFormat(
"EntityAsyncTask::HandleEndActionResult
[within thread]: Removing EntityMultiAsyncTasks object for entity '{0}'",
- entityInfo.getContainerId());
-
_multiTasksByEntities.remove(entityInfo.getContainerId());
- } else {
- entityInfo.resetActionTypeIfNecessary();
- entityInfo.StartPollingNextTask();
+ commandInfo.getCommandId());
+
_multiTasksByCommandIds.remove(commandInfo.getCommandId());
}
}
}
- }
}
catch (RuntimeException ex) {
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/EntityMultiAsyncTasks.java
index 198310f..91ba9c2 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/EntityMultiAsyncTasks.java
@@ -1,8 +1,5 @@
package org.ovirt.engine.core.bll;
-import java.util.List;
-
-import org.ovirt.engine.core.common.action.VdcActionType;
import org.ovirt.engine.core.common.asynctasks.EndedTaskInfo;
import org.ovirt.engine.core.common.asynctasks.EndedTasksInfo;
import org.ovirt.engine.core.compat.Guid;
@@ -10,50 +7,40 @@
import org.ovirt.engine.core.utils.log.LogFactory;
public class EntityMultiAsyncTasks {
- private VdcActionType privateActionType = VdcActionType.forValue(0);
-
- public VdcActionType getActionType() {
- return privateActionType;
- }
-
- public void setActionType(VdcActionType value) {
- privateActionType = value;
- }
private java.util.HashMap<Guid, EntityAsyncTask> _listTasks;
- private Object privateContainerId;
+ private Guid commandId;
- public Object getContainerId() {
- return privateContainerId;
+ public Guid getCommandId() {
+ return commandId;
}
- private void setContainerId(Object value) {
- privateContainerId = value;
+ private void setCommandId(Guid value) {
+ commandId = value;
}
- public EntityMultiAsyncTasks(Object containerID) {
+ public EntityMultiAsyncTasks(Guid commandId) {
_listTasks = new java.util.HashMap<Guid, EntityAsyncTask>();
- setContainerId(containerID);
+ setCommandId(commandId);
}
public void AttachTask(EntityAsyncTask asyncTask) {
synchronized (_listTasks) {
if (!_listTasks.containsKey(asyncTask.getVdsmTaskId())) {
- log.infoFormat("EntityMultiAsyncTasks::AttachTask: Attaching
task '{0}' to entity '{1}'.",
- asyncTask.getVdsmTaskId(), getContainerId());
+ log.infoFormat("EntityMultiAsyncTasks::AttachTask: Attaching
task '{0}' to command '{1}'.",
+ asyncTask.getVdsmTaskId(), getCommandId());
_listTasks.put(asyncTask.getVdsmTaskId(), asyncTask);
}
}
}
- private java.util.ArrayList<EntityAsyncTask> GetCurrentActionTypeTasks() {
+ private java.util.ArrayList<EntityAsyncTask> getCurrentTasks() {
java.util.ArrayList<EntityAsyncTask> retValue = new
java.util.ArrayList<EntityAsyncTask>();
for (EntityAsyncTask task : _listTasks.values()) {
if (task.getParameters() != null
&& task.getParameters().getDbAsyncTask() != null
- && task.getParameters().getDbAsyncTask().getaction_type()
== getActionType()
&& (task.getState() == AsyncTaskState.Polling ||
task.getState() == AsyncTaskState.Ended || task
.getState() ==
AsyncTaskState.AttemptingEndAction)) {
retValue.add(task);
@@ -65,7 +52,7 @@
public boolean ShouldEndAction() {
synchronized (_listTasks) {
- java.util.ArrayList<EntityAsyncTask> CurrentActionTypeTasks =
GetCurrentActionTypeTasks();
+ java.util.ArrayList<EntityAsyncTask> CurrentActionTypeTasks =
getCurrentTasks();
for (EntityAsyncTask task : CurrentActionTypeTasks) {
if (task.getState() != AsyncTaskState.Ended) {
@@ -79,7 +66,7 @@
public void MarkAllWithAttemptingEndAction() {
synchronized (_listTasks) {
- java.util.ArrayList<EntityAsyncTask> CurrentActionTypeTasks =
GetCurrentActionTypeTasks();
+ java.util.ArrayList<EntityAsyncTask> CurrentActionTypeTasks =
getCurrentTasks();
for (EntityAsyncTask task : CurrentActionTypeTasks) {
task.setState(AsyncTaskState.AttemptingEndAction);
@@ -92,7 +79,7 @@
java.util.ArrayList<EndedTaskInfo> endedTaskInfoList = new
java.util.ArrayList<EndedTaskInfo>();
synchronized (_listTasks) {
- java.util.ArrayList<EntityAsyncTask> CurrentActionTypeTasks =
GetCurrentActionTypeTasks();
+ java.util.ArrayList<EntityAsyncTask> CurrentActionTypeTasks =
getCurrentTasks();
for (EntityAsyncTask task : CurrentActionTypeTasks) {
task.setLastStatusAccessTime();
@@ -111,7 +98,7 @@
public int getTasksCountCurrentActionType() {
int returnValue = 0;
synchronized (_listTasks) {
- java.util.ArrayList<EntityAsyncTask> CurrentActionTypeTasks =
GetCurrentActionTypeTasks();
+ java.util.ArrayList<EntityAsyncTask> CurrentActionTypeTasks =
getCurrentTasks();
returnValue = CurrentActionTypeTasks.size();
}
@@ -120,7 +107,7 @@
public void Repoll() {
synchronized (_listTasks) {
- java.util.ArrayList<EntityAsyncTask> CurrentActionTypeTasks =
GetCurrentActionTypeTasks();
+ java.util.ArrayList<EntityAsyncTask> CurrentActionTypeTasks =
getCurrentTasks();
for (EntityAsyncTask task : CurrentActionTypeTasks) {
task.setState(AsyncTaskState.Ended);
@@ -128,54 +115,12 @@
}
}
- /**
- * Reset the action type if all the current action type's tasks are
cleared, so that if there are any other tasks
- * in the list they will get treated correctly.
- */
- protected void resetActionTypeIfNecessary() {
- boolean allCleared = true;
- synchronized (_listTasks) {
- List<EntityAsyncTask> CurrentActionTypeTasks =
GetCurrentActionTypeTasks();
-
- for (EntityAsyncTask task : CurrentActionTypeTasks) {
- allCleared = allCleared && taskWasCleared(task);
- }
-
- if (allCleared) {
- setActionType(VdcActionType.Unknown);
- }
- }
- }
-
public void ClearTasks() {
synchronized (_listTasks) {
- java.util.ArrayList<EntityAsyncTask> CurrentActionTypeTasks =
GetCurrentActionTypeTasks();
+ java.util.ArrayList<EntityAsyncTask> CurrentActionTypeTasks =
getCurrentTasks();
for (EntityAsyncTask task : CurrentActionTypeTasks) {
task.clearAsyncTask();
- }
-
- StartPollingNextTask();
- }
- }
-
- // call this method after ending action and clearing tasks of current
- // ActionType.
- protected void StartPollingNextTask() {
- synchronized (_listTasks) {
- for (EntityAsyncTask task : _listTasks.values()) {
- if (task.getState() == AsyncTaskState.WaitForPoll &&
task.getParameters() != null
- && task.getParameters().getDbAsyncTask() != null) {
- log.infoFormat(
- "EntityMultiAsyncTasks::StartPollingNextTask:
Starting to poll next task " +
- "(task ID: '{0}', action type '{1}')",
- task.getVdsmTaskId(),
-
task.getParameters().getDbAsyncTask().getaction_type());
-
- setActionType(VdcActionType.Unknown);
- StartPollingTask(task.getVdsmTaskId());
- break;
- }
}
}
}
@@ -184,44 +129,12 @@
synchronized (_listTasks) {
if (_listTasks.containsKey(TaskID) &&
_listTasks.get(TaskID).getParameters() != null
&& _listTasks.get(TaskID).getParameters().getDbAsyncTask()
!= null) {
- if (getActionType() == VdcActionType.Unknown) {
- // still no ActionType chosen -> determine the ActionType
by
- // the
- // TaskID that we want to start polling and start polling
- // all of
- // its siblings:
-
setActionType(_listTasks.get(TaskID).getParameters().getDbAsyncTask().getaction_type());
- log.infoFormat(
- "EntityMultiAsyncTasks::StartPollingTask: Current
Action Type for entity '{0}' is '{1}' (determined by task '{2}')",
- getContainerId(),
- getActionType(),
- TaskID);
-
- for (EntityAsyncTask task : _listTasks.values()) {
- if (task.getParameters() != null
- && task.getParameters().getDbAsyncTask() !=
null
- &&
task.getParameters().getDbAsyncTask().getaction_type() == getActionType()
- && (task.getState() ==
AsyncTaskState.Initializing || task.getState() == AsyncTaskState.WaitForPoll)) {
- task.setState(AsyncTaskState.Polling);
- }
+ for (EntityAsyncTask task : _listTasks.values()) {
+ if (task.getParameters() != null &&
task.getParameters().getDbAsyncTask() != null)
+ task.setState(AsyncTaskState.Polling);
}
- }
-
- else {
- // ActionType is already determined -> set the task and its
- // siblings
- // to 'wait for poll':
- VdcActionType currTaskActionType =
_listTasks.get(TaskID).getParameters().getDbAsyncTask()
- .getaction_type();
- for (EntityAsyncTask task : _listTasks.values()) {
- if (task.getParameters() != null &&
task.getParameters().getDbAsyncTask() != null
- &&
task.getParameters().getDbAsyncTask().getaction_type() == currTaskActionType
- && task.getState() ==
AsyncTaskState.Initializing) {
- task.setState(AsyncTaskState.WaitForPoll);
- }
- }
- }
- } else {
+ }
+ else {
log.warnFormat(
"EntityMultiAsyncTasks::StartPollingTask: For some
reason, task '{0}' has no parameters or no DB task - we don't know its action
type",
TaskID);
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SPMAsyncTask.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SPMAsyncTask.java
index 7b40810..a823e47 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SPMAsyncTask.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SPMAsyncTask.java
@@ -107,8 +107,8 @@
return _lastAccessToStatusSinceEnd;
}
- public Object getContainerId() {
- return getParameters().getEntityInfo().getId();
+ public Guid getCommandId() {
+ return getParameters().getDbAsyncTask().getRootCommandId();
}
public void StartPollingTask() {
--
To view, visit http://gerrit.ovirt.org/15569
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I53999748c791aae04287efaa01c10ea7037e5ff2
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
