Re: Glusterd daemon management code refactoring

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



While modelling daemons as proposed here, I noticed that I didn't
foresee thathow abstract data types and embedded structures
(think struct list_head) 

----- Original Message -----
> 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@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@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@xxxxxxxxxxx
> > >> http://supercolony.gluster.org/mailman/listinfo/gluster-devel
> > >>
> > > _______________________________________________
> > > Gluster-devel mailing list
> > > Gluster-devel@xxxxxxxxxxx
> > > http://supercolony.gluster.org/mailman/listinfo/gluster-devel
> > > 
> > ~Atin
> > 
> _______________________________________________
> Gluster-devel mailing list
> Gluster-devel@xxxxxxxxxxx
> http://supercolony.gluster.org/mailman/listinfo/gluster-devel
> 
_______________________________________________
Gluster-devel mailing list
Gluster-devel@xxxxxxxxxxx
http://www.gluster.org/mailman/listinfo/gluster-devel





[Index of Archives]     [Gluster Users]     [Ceph Users]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux