[Gluster-devel] Glusterd daemon management code refactoring

[Krishnan Parthasarathi]

Here is the first patch, http://review.gluster.org/9278, which marks the beginning
of the implementation phase of this refactoring effort. Reviews/comments for the (design) proposal
helped us to correct a few things and kick start coding. Look forward to see the same enthusiasm
with the code review(s) :-)


----- Original Message -----
> 
> 
> On 12/09/2014 06:24 PM, Jeff Darcy wrote:
> >> I would like to propose refactoring of the code managing
> >> various daemons in glusterd. Unlike other high(er) level proposals
> >> about feature design, this one is at the implementation
> >> level. Please go through the details of the proposal below
> >> and share your thoughts/suggestions on the approach.
> > 
> > I like this idea, as a way of reducing technical debt.  Can we take it a
> > step further, and provide default *implementations* for some of these
> > methods?  For example, connect/disconnect and is_running are likely to
> > be the same for practically all daemons.
> We do have a plan to implement the default implementations.
> 
> > 
> >> ### Introduction
> >>
> >> Glusterd manages GlusterFS daemons providing services like NFS, Proactive
> >> self-heal, Quota, User servicable snapshots etc. Following are some of the
> >> aspects that come under daemon management.
> >>
> >> - Connection Management
> >>   - unix domain sockets based channel for internal communication
> >>   - Methods - connect, disconnect, notify
> >>
> >> - Process Management
> >>   - pidfile to detect if the daemon is running
> >>   - Environment; run-dir, svc-dir, log-dir etc.
> >>   - Methods - start, stop, status, kill
> >>
> >> - Daemon-specific Management
> >>
> >> Currently, the daemon management code is fragmented and doesn't elicit the
> >> structure described above. This results in further fragmentation since new
> >> developers may not identify common patterns, worse even, they won't be
> >> able
> >> to
> >> do anything about it.
> >>
> >> This proposal aims to do the following,
> >>
> >> - Provide an abstract data type that encapsulates what is common among
> >> daemons
> >>   that are managed by glusterd.
> >>
> >> - 'Port' existing code to make use of the abstract type. This would help
> >> in
> >>   making this change self-documented to an extent.
> >>
> >> - Prescribe a way to maintain per-feature daemon code separate to
> >> glusterd's
> >>   common code.
> >>
> >> ### Abstract data types
> >>
> >>         struct conn_mgmt {
> >>             struct rpc_clnt *rpc;
> >>             int (*connect) (struct conn_mgmt *self);
> >>             int (*disconnect) (struct conn_mgmt self);
> >>             int (*notify) (struct conn_mgmt *self, rpc_clnt_event_t
> >>             *rpc_event);
> >>         }
> >>
> >>         struct proc_mgmt {
> >>             char svcdir[PATH_MAX];
> >>             char rundir[PATH_MAX];
> >>             char logdir[PATH_MAX];
> >>             char pidfile[PATH_MAX];
> >>             char logfile[PATH_MAX];
> >>
> >>             char volid[UUID_CANONICAL_FORM_LEN];
> > 
> > How about a PID, so that default implementations (e.g. is_running
> > or kill) can use it?
> pidfile will have the details for it.
> 
> > 
> >>             int (*start) (struct proc_mgmt *self, int flags);
> >>             int (*stop) (struct proc_mgmt *self, int flags);
> >>             int (*is_running) (struct proc_mgmt *self);
> >>             int (*kill) (struct proc_mgmt *self, int flags);
> >>
> >>         }
> >>
> >> Feature authors can define data type representing their service by
> >> implementing
> >> the above 'abstract' class. For e.g,
> >>
> >>         struct my_service {
> >>             char name[PATH_MAX];
> >>             /* my_service specific data members and methods */
> >>
> >>             /* The methods in the following structures should be
> >>             implemented
> >>             by
> >>                respective feature authors */
> >>
> >>             struct conn_mgmt conn;
> >>             struct proc_mgmt proc;
> >>         }
> >>
> >> ### Code structure guidelines
> >>
> >> Each feature that introduces a daemon would implement the abstract data
> >> type.
> >> The implementations should be in separate files named appropriately. The
> >> intent
> >> is to avoid feature specific code to leak into common glusterd codebase.
> >> glusterd-utils.c is testament to such practices in the past.
> >>
> >> For e.g,
> >> [kp at trantor glusterd]$ tree
> >> .
> >> └── src
> >>     ├── glusterd-conn-mgmt.c
> >>     ├── glusterd-conn-mgmt.h
> >>     ├── glusterd-proc-mgmt.c
> >>     ├── glusterd-proc-mgmt.h
> >>     ├── my-feature-service.c
> >>     └── my-feature-service.h
> >>
> >> [kp at trantor glusterd]$ cat src/my-feature-service.h
> >>         #include "glusterd-conn-mgmt.h"
> >>         #include "glusterd-proc-mgmt.h"
> >>
> >> ...
> >> [rest of the code elided]
> >>
> >> ### Bibliography
> >>
> >> - Object-oriented design patterns in the kernel, part 1 -
> >> http://lwn.net/Articles/444910/
> >> - Object-oriented design patterns in the kernel, part 2 -
> >> http://lwn.net/Articles/446317/
> >>
> >>
> >> thanks,
> >> kp
> >> _______________________________________________
> >> Gluster-devel mailing list
> >> Gluster-devel at gluster.org
> >> http://supercolony.gluster.org/mailman/listinfo/gluster-devel
> >>
> > _______________________________________________
> > Gluster-devel mailing list
> > Gluster-devel at gluster.org
> > http://supercolony.gluster.org/mailman/listinfo/gluster-devel
> > 
> ~Atin
>