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
