[Gluster-devel] Glusterd daemon management code refactoring
Krishnan Parthasarathi
kparthas at redhat.com
Wed Jan 21 09:22:55 UTC 2015
[Apologies for sending an incomplete message]
While implementing daemon management code as proposed here, I found it wasn't
possible to embed structures that are abstract data types (ADT). In short,
ADT's 'real' size is unknown to the containing type and compilation fails. This
brings us to which one of the two is important.
Embedded structures allows us to model daemons in a compositional style lending
clarity of purpose to each component. For e.g any daemon management could be
decomposed into,
- managing the daemon process' lifecycle
- maintaining IPC with the daemon
- managing daemon's file system environment
With embedded structures, we don't have to allocate memory individually. This
simplifies the implementation.
While ADT would protect consumers from changes to the type, like adding a
member, it could lead to a lot of getters and setters (boiler-plate). With
these observations, I choose to go with embedded structures approach, leaving
ADTs for a different purpose elsewhere. Thoughts?
~kp
----- Original Message -----
> 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 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
> > >
> > _______________________________________________
> > 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://www.gluster.org/mailman/listinfo/gluster-devel
>
More information about the Gluster-devel
mailing list