[Gluster-Maintainers] Compound patches waiting for merging in release-3.8

[Krutika Dhananjay]

The patch @ http://review.gluster.org/#/c/15716/1 introduces whatever was
asked for - a test script that enables compound fops and exercises the
affected codepath. Note that the script also caught some bugs in the code
which
have been fixed as part of the same patch.

@Niels - Request you to revoke the -2s on the 4 existing compound fops
patches and have it merged once the
final patch (the one that introduces the script) passes regression.

-Krutika

On Thu, Sep 15, 2016 at 2:40 PM, Niels de Vos <ndevos at redhat.com> wrote:

> On Wed, Sep 07, 2016 at 10:42:15PM +0530, Pranith Kumar Karampuri wrote:
> > On Wed, Sep 7, 2016 at 10:21 PM, Pranith Kumar Karampuri <
> > pkarampu at redhat.com> wrote:
> >
> > >
> > >
> > > On Wed, Sep 7, 2016 at 8:00 PM, Niels de Vos <ndevos at redhat.com>
> wrote:
> > >
> > >> Hi Krutika and Pranith,
> > >>
> > >> The patches that introduce the compound operations for release-3.8 are
> > >> almost ready for merging. Is there a particular order for the merging
> to
> > >> be done? I'd hate to break git-bisect if I do it in the wrong order.
> > >>
> > >>   http://review.gluster.org/#/q/status:open+project:glusterfs+
> > >> branch:release-3.8+topic:bug-1372693
> > >>
> > >> These massive changes +1500 lines of code, have not one test.
> Eventhough
> > >> a new volume option is introduced. I really do hope there is some test
> > >> in the master branch that runs soemthing useful with the new option
> > >> enabled. Could you please backport that test-case too?
> > >>
> > >
> > > These patches are stabilization of experimental feature compound-fops
> for
> > > 3.8.x that is why you see massive changes because it is still
> stabilizing.
> > > As per the guidelines it is okay to do this. As this is performance
> > > enhancement, the nature of tests we did were manual and comparing the
> time
> > > elapsed with and without the fixes. We see 10% performance improvement
> > > overall in small file create workload where the write workload is 40%
> where
> > > the enhancement comes into picture. That is the reason you don't see
> any
> > > automated test as I don't think we can reliably test the performance
> > > improvement at the moment in automation. May be we can integrate it
> with
> > > the one Nigel & Shyam are driving once it is available.
> > >
> > >
> > >>
> > >> Because this adds a new functionality based on the (experimental)
> > >> compount FOPs, we need to mention it in the release notes. Feel free
> to
> > >> start a new doc/release-notes/3.8.x.md with a little text about the
> > >> goal of feature, how users can enable it and verify results.
> > >>
> > >
> > > Hmm... it is not a feature, it is stabilization of feature and using
> it.
> > > We can probably add a short description of how the option needs to be
> used
> > > to take advantage of the perf enhancement in the bz. It can be taken as
> > > part of the normal process of aggregating release notes.
> > >
> > >
> > >>
> > >> I really want us to prevent introducing such a lot of new code, that
> > >> does not go through regular testing. Should this be included in 3.8.4,
> > >> or maybe the release after it?
> > >>
> > >
> > > For it to go through regular testing, by default the option
> > > use-compound-fops needs to be enabled, but since all of this is still
> > > experimental, I would like it to be disabled. What are the tentative
> > > release dates for 3.8.4/3.8.5?
> > >
> >
> > I see that you plan to release it sometime next week. I think it is safer
> > to merge these just after the tagging so that it will go to 3.8.5. It
> will
> > also lessen the probability of surprises.
>
> Yes, these patches are candidates for 3.8.5. However, I *REALLY* demand
> some minimal automated testing. At the very least, enable the option and
> do a few operations that use the COMPOUND procedures.
>
> Thanks,
> Niels
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.gluster.org/pipermail/maintainers/attachments/20161025/73738f53/attachment.html>