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

Reply via email to