Ravi Nori has posted comments on this change. Change subject: engine : Introduction of CallBack interface ......................................................................
Patch Set 1: (3 comments) http://gerrit.ovirt.org/#/c/26333/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandsFactory.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandsFactory.java: Line 41: Line 42: private static CommandsFactory instance = new CommandsFactory(); Line 43: Line 44: public static CommandsFactory getInstance() { Line 45: return instance; > Why the singletone? I will remove the singleton Line 46: } Line 47: Line 48: private static ConcurrentMap<String, Class<CommandBase<? extends VdcActionParametersBase>>> commandsCache = Line 49: new ConcurrentHashMap<String, Class<CommandBase<? extends VdcActionParametersBase>>>(VdcActionType.values().length); http://gerrit.ovirt.org/#/c/26333/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/AsyncTaskManager.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/AsyncTaskManager.java: Line 72: private Map<Guid, AsyncTasks> partiallyCompletedCommandTasks = new ConcurrentHashMap<>(); Line 73: Line 74: private CountDownLatch irsBrokerLatch; Line 75: Line 76: private static AsyncTaskManager taskManager; > should be private static volatile AsyncTaskManager taskManager; will change Line 77: private static final Object LOCK = new Object(); Line 78: private CommandCoordinator coco; Line 79: Line 80: public static AsyncTaskManager getInstance(CommandCoordinator coco) { http://gerrit.ovirt.org/#/c/26333/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/interfaces/CommandCoordinator.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/interfaces/CommandCoordinator.java: Line 1: package org.ovirt.engine.core.bll.tasks.interfaces; Line 2: Line 3: public abstract class CommandCoordinator implements CallBack { > Why is this needed? At each subsequent patch CommandCoordinator implements more and more interfaces and finally looks like the class below. Instead of adding all the methods into this class, I created interfaces that hold the functions. CommandCoordinatorImpl extends CommandCoordinator and will implement all the functions needed. Please suggest an alternative if you have one in mind public abstract class CommandCoordinator implements CallBack, TaskHelper, Poller { public abstract void persistCommand(Guid commandId, Guid parentCommandId, VdcActionType actionType, VdcActionParametersBase params); public abstract CommandBase<?> retrieveCommand(Guid commandId); } -- To view, visit http://gerrit.ovirt.org/26333 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8c12315c96168fd32dc05ac6de336ccdd63c9fbc Gerrit-PatchSet: 1 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: 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
