Shireesh Anjal has posted comments on this change. Change subject: engine: Refactoring GlusterManager ......................................................................
Patch Set 2: (1 inline comment) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterSyncManager.java Line 55: * This class is responsible for keeping the Gluster related data of engine in sync with the actual data retrieved from Line 56: * GlusterFS. This helps to make sure that any changes done on Gluster servers using the Gluster CLI are propagated to Line 57: * engine as well. Line 58: */ Line 59: public class GlusterSyncManager extends GlusterJobsManager { While I like the idea of having a single gluster jobs manager, I think the actual jobs should not extend from it. IMO something like following is a better idea: - GlusterJobsManager that schedules all gluster related background "jobs" (almost same as what you have created) - An abstract base class GlusterJob that provides common functionality for all our jobs (Some of the methods you have kept in GlusterJobsManager will then move to this class) - Each individual job class can extend from GlusterJob, including this one, which should be renamed to GlusterSyncJob Line 60: private final Log log = LogFactory.getLog(GlusterSyncManager.class); Line 61: private final LockManager lockManager = LockManagerFactory.getLockManager(); Line 62: private GlusterAuditLogUtil logUtil = GlusterAuditLogUtil.getInstance(); Line 63: private static final GlusterSyncManager instance = new GlusterSyncManager(); -- To view, visit http://gerrit.ovirt.org/13944 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I61d851928f18da22b352b7be9d6ccaf4382e7930 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Sahina Bose <sab...@redhat.com> Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com> Gerrit-Reviewer: Sahina Bose <sab...@redhat.com> Gerrit-Reviewer: Shireesh Anjal <san...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches