Re: [Gluster-devel] [Gluster-Maintainers] RFC: Gluster.Next: Where and how DHT2 work/code would be hosted
On 10/11/2015 06:09 PM, Niels de Vos wrote: On Fri, Oct 09, 2015 at 10:40:15AM -0400, Shyam wrote: On 10/09/2015 12:07 AM, Atin Mukherjee wrote: First of all my apologies for not going through the meeting blog before sending my thoughts on how we plan to maintain GlusterD 2.0 [1]. This approach seems fine to me as long as we don't touch any existing xlators. How do we handle cases where other xlators get impacted by certain changes. Are we going to copy the whole translator in xlators/experimental and start working on it? Nope, we should send a change request for that xlator as a separate commit when possible. The counter example to this is, point (4) below (where DHT2 needs a bit of change in glusterd, but ...). I suggest such changes be maintained as .patch files inside the xlator, till a point when this can be merged is decided. No, please not, this makes it practically impossible to track the history of the changes. See this other email that suggests using different functions based on a #define: http://www.gluster.org/pipermail/gluster-devel/2015-October/046889.html This is clean, does not need nasty hacks in the build system and keeps the history. Also, when someone changes one variation of the function, the other variation would get corrected at the same time (or the reviewers point that missing bit out). The #if/else approach works for me, the only problem is that we should not pollute the supported code too much, that it becomes difficult to maintain, or newer functionality causes unstable behaviour. Instead of all this wouldn't it be simpler to have development under a separate branch say "4.0-unstable" and we could disable CI on this branch till it becomes stable? Are we worried about pulling in the changes from this to master once the branch becomes stable? I guess the worry is *bulk* changes appearing in master (as per meeting minutes). I share the same concern as well (on bulk changes), but I am unsure of review stringency on experimental, as things will evolve here, than each commit be ready for a clean review from day 1. So, this is an open confusion in my head as well, as when we want to move an xlator from experimental to suported, what would be the criteria? Would we not be doing bulk reviews then as well? Changes for a new feature should also be done in steps that can reasonably be reviewed, so, smaller patches addressing a particular functionality. Developers that are interested in the feature, should have a good review process. The standards that we apply for non-experimental changes are strict, but is there really a good reason to not apply those guidelines for patches to expiremental xlators too? There are 2 sub-problems here, and I think I answered on of them in another mail, which is *bulk* changes. The short of it being, yes there would be incremental changes to the experimental code base, but *when* it has to move out of experimental, that move may need a bulk review, we cannot get away from this in any scheme, and with code in master, there is a good chance that, as the xlator matures more eyes are called in to review the changes, so the bulk point is rather not as bulky and should become moot. IOW, *bulk* is no longer a concern (at least for me with the master branch+experimental approach). The second issue I see here is the quality of each submission and the review process around it etc. I would answer it this way, code that compiles may not work or could have issues during integration, or things under experimental would change course during its lifetime. As a result, some experimental changes would be allowed, and hence move forward the development or thought process, to a point where we are in functioning order at some time. Having a very stringent review and working order code in experimental, could hamper development pace. I guess each experimental xlator should have a TODO list in its sources. If a reviewer notices some issue, it may be acceptible to add the problem to the TODO list (with a pointer to the review URL where it was spotted). This allows fast progress, and we have some criteria that gets collected with which we can decide if the xlator may move out of experimental. It also makes it possible for others to update the TODO list when reviews of the already merged code is done. This is better, I was using TODO in code for the same, a list of issues is better to track, close, check etc. I will add this to the criteria in the commit for the same. Niels What do others think? This is just my thought and I would like to get a clarity on this. Thanks, Atin [1] http://www.gluster.org/pipermail/gluster-devel/2015-October/046872.html On 10/08/2015 11:35 PM, Shyam wrote: Hi, On checking yesterday's gluster meeting AIs and (later) reading the minutes, for DHT2 here is what I gather and propose to do for $SUBJECT. Feel free to add/negate any plans. (This can also be discussed at [2])
Re: [Gluster-devel] [Gluster-Maintainers] RFC: Gluster.Next: Where and how DHT2 work/code would be hosted
On Fri, Oct 09, 2015 at 10:40:15AM -0400, Shyam wrote: > On 10/09/2015 12:07 AM, Atin Mukherjee wrote: > >First of all my apologies for not going through the meeting blog before > >sending my thoughts on how we plan to maintain GlusterD 2.0 [1]. > > > >This approach seems fine to me as long as we don't touch any existing > >xlators. How do we handle cases where other xlators get impacted by > >certain changes. Are we going to copy the whole translator in > >xlators/experimental and start working on it? > > Nope, we should send a change request for that xlator as a separate commit > when possible. > > The counter example to this is, point (4) below (where DHT2 needs a bit of > change in glusterd, but ...). > > I suggest such changes be maintained as .patch files inside the xlator, till > a point when this can be merged is decided. No, please not, this makes it practically impossible to track the history of the changes. See this other email that suggests using different functions based on a #define: http://www.gluster.org/pipermail/gluster-devel/2015-October/046889.html This is clean, does not need nasty hacks in the build system and keeps the history. Also, when someone changes one variation of the function, the other variation would get corrected at the same time (or the reviewers point that missing bit out). > >Instead of all this wouldn't it be simpler to have development under a > >separate branch say "4.0-unstable" and we could disable CI on this > >branch till it becomes stable? Are we worried about pulling in the > >changes from this to master once the branch becomes stable? > > I guess the worry is *bulk* changes appearing in master (as per meeting > minutes). I share the same concern as well (on bulk changes), but I am > unsure of review stringency on experimental, as things will evolve here, > than each commit be ready for a clean review from day 1. So, this is an open > confusion in my head as well, as when we want to move an xlator from > experimental to suported, what would be the criteria? Would we not be doing > bulk reviews then as well? Changes for a new feature should also be done in steps that can reasonably be reviewed, so, smaller patches addressing a particular functionality. Developers that are interested in the feature, should have a good review process. The standards that we apply for non-experimental changes are strict, but is there really a good reason to not apply those guidelines for patches to expiremental xlators too? I guess each experimental xlator should have a TODO list in its sources. If a reviewer notices some issue, it may be acceptible to add the problem to the TODO list (with a pointer to the review URL where it was spotted). This allows fast progress, and we have some criteria that gets collected with which we can decide if the xlator may move out of experimental. It also makes it possible for others to update the TODO list when reviews of the already merged code is done. Niels > > What do others think? > > > > >This is just my thought and I would like to get a clarity on this. > > > >Thanks, > >Atin > > > >[1] http://www.gluster.org/pipermail/gluster-devel/2015-October/046872.html > > > >On 10/08/2015 11:35 PM, Shyam wrote: > >>Hi, > >> > >>On checking yesterday's gluster meeting AIs and (later) reading the > >>minutes, for DHT2 here is what I gather and propose to do for $SUBJECT. > >>Feel free to add/negate any plans. > >> > >>(This can also be discussed at [2]) > >> > >>--- > >>1) Create a directory under the glusterfs master branch as follows, > >>./xlators/*experimental*/dht2 > >>./xlators/*experimental*/posix2 > >> > >>See patch request at [2] > >> > >>All code, design documents (work products in general) would go into this > >>directory. > >> > >>2) Code that compiles and does not cause CI failures could *potentially* > >>be merged with very few DHT2 dev folks assent. > >> > >>There would possibly be no CI integration till we get something working, > >>so merges would be based on compile passing initially. Soon there would > >>be an attempt at getting unit testing integrated, so that code being > >>submitted is not abysmally horrendous > >> > >>3) Common framework code changes (if any) would be presented as a > >>separate commit request > >> > >>4) (Big problem) DHT2 requires glusterd changes to create a volume as > >>DHT2 and not DHT, this would be maintained as a .patch in the dht2 > >>directory as above. This is so that people can play with DHT2 volumes if > >>interested. Integration of this piece either comes with glusterd 2.0 or > >>based on time lines of other events, in the current version of glusterd. > >>(if you are interested in seeing the current version of this patch, go > >>here [1]) > >>--- > >> > >>If there is some key disagreement on certain points like (2) above, then > >>we would need to bring in DHT
Re: [Gluster-devel] [Gluster-Maintainers] RFC: Gluster.Next: Where and how DHT2 work/code would be hosted
On Fri, Oct 09, 2015 at 09:37:11AM +0530, Atin Mukherjee wrote:
> First of all my apologies for not going through the meeting blog before
> sending my thoughts on how we plan to maintain GlusterD 2.0 [1].
Because GlusterD-2.0 is quite a different approach, I am not sure it
should get included in the main glusterfs repository. At least for the
time being. If there is any tight integration, then I feel it should get
included, but if the new GlusterD 'only' prepares, starts and stops
processes, I think it can stay external.
I like a component based setup, where we can easily replace/update
single parts and are not required to update all running processes.
A patch to make GlusterD optional in the current repository should
probably be sent sooner than later. With such a patch, a pointer to
instructions that explain how to run with the new GlusterD would be a
requirement.
> This approach seems fine to me as long as we don't touch any existing
> xlators. How do we handle cases where other xlators get impacted by
> certain changes. Are we going to copy the whole translator in
> xlators/experimental and start working on it?
For major changes maybe. I prefer to see all the 'touching' done on the
normal xlators. configure.ac should set an EXPERIMENTAL (and maybe per
expiremental xlator?) #define in config.h. This #define can then be used
to #if/#else the 'touching' of the code. Using empty and non-empty
functions for these changes would be nice, that should keep the code
pretty readable, lots of #ifdef'ing in a function makes it difficult to
understand what the code is doing.
#ifndef EXPERIMENTAL
static void do_it (void) {}
#else /* */
static void do_it (void)
{
printf ("Hello Experiment!\n");
}
#endif
void main (void)
{
do_it ();
}
> Instead of all this wouldn't it be simpler to have development under a
> separate branch say "4.0-unstable" and we could disable CI on this
> branch till it becomes stable? Are we worried about pulling in the
> changes from this to master once the branch becomes stable?
This can be an option, but then I would rather see a branch per feature.
Whe na feature is ready, all its changes should get merged into the
master branch. I do not expect the different features to be ready at the
same time. Also regular re-basing of the feature branches on top of the
master branch would need to be done to prevent feature branches getting
out of sync.
This is a little similar to what linux-next does too. Many different
branches with new features and bug fixes get merged into a daily
linux-next. Merge conflicts get detected early, and people can test
linux-next and report problems that the features introduced.
HTH,
Niels
>
> This is just my thought and I would like to get a clarity on this.
>
> Thanks,
> Atin
>
> [1] http://www.gluster.org/pipermail/gluster-devel/2015-October/046872.html
>
> On 10/08/2015 11:35 PM, Shyam wrote:
> > Hi,
> >
> > On checking yesterday's gluster meeting AIs and (later) reading the
> > minutes, for DHT2 here is what I gather and propose to do for $SUBJECT.
> > Feel free to add/negate any plans.
> >
> > (This can also be discussed at [2])
> >
> > ---
> > 1) Create a directory under the glusterfs master branch as follows,
> > ./xlators/*experimental*/dht2
> > ./xlators/*experimental*/posix2
> >
> > See patch request at [2]
> >
> > All code, design documents (work products in general) would go into this
> > directory.
> >
> > 2) Code that compiles and does not cause CI failures could *potentially*
> > be merged with very few DHT2 dev folks assent.
> >
> > There would possibly be no CI integration till we get something working,
> > so merges would be based on compile passing initially. Soon there would
> > be an attempt at getting unit testing integrated, so that code being
> > submitted is not abysmally horrendous
> >
> > 3) Common framework code changes (if any) would be presented as a
> > separate commit request
> >
> > 4) (Big problem) DHT2 requires glusterd changes to create a volume as
> > DHT2 and not DHT, this would be maintained as a .patch in the dht2
> > directory as above. This is so that people can play with DHT2 volumes if
> > interested. Integration of this piece either comes with glusterd 2.0 or
> > based on time lines of other events, in the current version of glusterd.
> > (if you are interested in seeing the current version of this patch, go
> > here [1])
> > ---
> >
> > If there is some key disagreement on certain points like (2) above, then
> > we would need to bring in DHT2 code in parts so that it makes sense.
> > This is fine too, just that we would have 2 repos till we reach a point
> > of maturity in development.
> >
> > ---
> > *Some issues with the approach:*
> > A) We need
