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
