[Gluster-devel] Glusterd daemon management code refactoring

[Jeff Darcy]

> 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.

> ### 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?

>             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
>