Yair Zaslavsky has posted comments on this change.
Change subject: 15. core : Handle incomplete tasks on server restart
......................................................................
Patch Set 6: (4 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AsyncTaskManager.java
Line 71: /**
Line 72: * Map of tasks in DB per storage pool that exist after restart *
Line 73: */
Line 74: private ConcurrentMap<Guid, List<AsyncTasks>>
tasksInDbAfterRestart = null;
Line 75: private List<AsyncTasks> partiallyCompletedCommandTasks = new
ArrayList<>();
I didn't see any place where order is essential. Consider Map<Guid,AsyncTasks>
if this collection is truely needed, see comments below (I may have repeated
myself).
Line 76: private static final AsyncTaskManager _taskManager = new
AsyncTaskManager();
Line 77:
Line 78: public static AsyncTaskManager getInstance() {
Line 79: return _taskManager;
Line 300: return false;
Line 301: }
Line 302:
Line 303: private AsyncTasks getPartiallyCompletedTaskFromList(Guid vdsmId)
{
Line 304: for (AsyncTasks task : partiallyCompletedCommandTasks) {
Why not using a map instead of list?
Line 305: if (vdsmId.equals(task.getVdsmTaskId())) {
Line 306: return task;
Line 307: }
Line 308: }
Line 698: if
(!_tasks.containsKey(creationInfo.getVdsmTaskId())) {
Line 699: try {
Line 700: SPMAsyncTask task;
Line 701: if
(isPartiallyCompletedCommandTask(creationInfo.getVdsmTaskId())) {
Line 702: AsyncTasks asyncTaskInDb =
getPartiallyCompletedTaskFromList(creationInfo.getVdsmTaskId());
As asked before, can't we just check if vdsmTaskId is null , and not use the
collection that holds partially completed list?
Line 703: task =
AsyncTaskFactory.Construct(creationInfo, asyncTaskInDb);
Line 704: if (task.getEntitiesMap() == null) {
Line 705: task.setEntitiesMap(new
HashMap<Guid, VdcObjectType>());
Line 706: }
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SPMAsyncTask.java
Line 478: }
Line 479:
Line 480: private static final Log log =
LogFactory.getLog(SPMAsyncTask.class);
Line 481:
Line 482: private boolean isPartiallyCompletedCommandTask = false;
should be renamed to partiallyCompletedCommandTask, and getter should be
isPartially... and not isIs....
Setter should be setPartiallyCompleted.....
But a question here - why do I need this field at all? the AsyncTasks object
can be obtained from the AsyncTaskParameters, so you can have a boolean method
named "isMissingVdsmTaskId()" which will check this.
What do you think?
Line 483:
Line 484: public boolean isIsPartiallyCompletedCommandTask() {
Line 485: return isPartiallyCompletedCommandTask;
Line 486: }
--
To view, visit http://gerrit.ovirt.org/15630
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a979d816d9ec8cf25119a33742c8e6af4ff42a8
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
