Ravi Nori has posted comments on this change. Change subject: engine : Modify Async_tasks table to use CommandEntity ......................................................................
Patch Set 14: (6 comments) http://gerrit.ovirt.org/#/c/26687/14/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandCoordinatorImpl.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandCoordinatorImpl.java: Line 136: public void updateCommandStatus(final Guid commandId, final AsyncTaskType taskType, final CommandStatus status) { Line 137: commandsCache.updateCommandStatus(commandId, taskType, status); Line 138: } Line 139: Line 140: public List<AsyncTasks> getAllAsyncTasksFromDb() { > Hi, just a thought here, All this will disappear into Async tasks CommandCallBack in the future. I can create a helper class for now but it is another redirection Line 141: List<AsyncTasks> asyncTasks = DbFacade.getInstance().getAsyncTaskDao().getAll(); Line 142: for (AsyncTasks asyncTask : asyncTasks) { Line 143: copyFromCommandEntity(asyncTask, commandsCache.get(asyncTask.getCommandId())); Line 144: } Line 170: /** Line 171: * This method is always called from with in a transaction Line 172: * @param asyncTask Line 173: */ Line 174: public void saveAsyncTaskToDb(final AsyncTasks asyncTask) { > Hi, Will wrap and see how it works Line 175: DbFacade.getInstance().getAsyncTaskDao().save(asyncTask); Line 176: persistCommand(copyToCommandEntity(asyncTask, commandsCache.get(asyncTask.getCommandId()))); Line 177: } Line 178: Line 178: Line 179: public AsyncTasks getAsyncTaskFromDb(Guid asyncTaskId) { Line 180: AsyncTasks asyncTask = DbFacade.getInstance().getAsyncTaskDao().get(asyncTaskId); Line 181: if (asyncTask != null) { Line 182: copyFromCommandEntity(asyncTask, commandsCache.get(asyncTask.getCommandId())); > instead of copyFrom I prefer the term "map" - as someone who worked alot on Will change Line 183: } Line 184: return asyncTask; Line 185: } Line 186: http://gerrit.ovirt.org/#/c/26687/14/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandsCacheImpl.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandsCacheImpl.java: Line 21: public CommandsCacheImpl() { Line 22: commandMap = CacheProviderFactory.<Guid, CommandEntity> getCacheWrapper(COMMAND_MAP_NAME); Line 23: } Line 24: Line 25: public void initializeCache() { > any point of of having this "initializeCache" method not private and called I can change it to private, did not want to go to DB in CTOR. Lazy initialization, is that okay? Line 26: if (!cacheInitialized) { Line 27: synchronized(LOCK) { Line 28: if (!cacheInitialized) { Line 29: List<CommandEntity> cmdEntities = DbFacade.getInstance().getCommandEntityDao().getAll(); http://gerrit.ovirt.org/#/c/26687/14/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/SPMAsyncTask.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/SPMAsyncTask.java: Line 244: } Line 245: Line 246: protected void RemoveTaskFromDB() { Line 247: try { Line 248: if (coco.removeByVdsmTaskId(getVdsmTaskId()) != 0) { > imho != 0 is bad... and doesn't tell much the code reviewer or any future r Did not change logic here from current master, but you are right will fix it. Line 249: log.infoFormat("BaseAsyncTask::RemoveTaskFromDB: Removed task {0} from DataBase", getVdsmTaskId()); Line 250: } Line 251: } Line 252: http://gerrit.ovirt.org/#/c/26687/14/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/interfaces/AsyncTaskCRUDOperations.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/interfaces/AsyncTaskCRUDOperations.java: Line 5: Line 6: import java.util.List; Line 7: Line 8: public interface AsyncTaskCRUDOperations { Line 9: public abstract List<AsyncTasks> getAllAsyncTasksFromDb(); > abstract is not needed. Will remove it. Line 10: public abstract void saveAsyncTaskToDb(AsyncTasks asyncTask); Line 11: public abstract AsyncTasks getAsyncTaskFromDb(Guid asyncTaskId); Line 12: public abstract int removeTaskFromDbByTaskId(Guid taskId) throws RuntimeException; Line 13: public abstract AsyncTasks getByVdsmTaskId(Guid vdsmTaskId); -- To view, visit http://gerrit.ovirt.org/26687 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ied038b514f1285b4d46deff022beafd9dcfe5c29 Gerrit-PatchSet: 14 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Greg Padgett <gpadg...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
