geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jason Huynh <jhu...@pivotal.io>
Subject Re: [DISCUSS] Clean build takes 10minutes to complete now
Date Fri, 15 Sep 2017 20:08:25 GMT
For the original issue, where the old version is pulling down the old
versions during compilation time, we have a pull request to use the maven
repo: https://github.com/apache/geode/pull/790

The rest of the tests added for the session state are marked as @Category
({DistributedTest.class, BackwardCompatibilityTest.class})
 geode-old-versions happens to be required at compile time.

The old versions will now (after the pull request is checked in) pull down
the zip into the local repo once.  Unzipping will still be required but
that should be a lot shorter than downloading.

Whether we should move the old versions out of the compile task is a
different issue...



On Fri, Sep 15, 2017 at 8:55 AM Kirk Lund <klund@apache.org> wrote:

> The actual tests marked with UnitTest category are actually pretty good.
> They all run in just over one minute and almost all of them use Mockito to
> isolate one class. I remember seeing one newer Lucene UnitTest that touches
> File System which should be recategorized as IntegrationTest.
>
> If we could move the pulling down of previous versions of Geode out of the
> main build+unit-test target, that would help a lot.
>
> Even prior to the pulling down of previous versions for backwards compat
> testing, the main build (without unit-test) was too slow and I think it's
> because our project is a little too complex for what Gradle is designed to
> handle.
>
> Code generation and javadocs are two of the tasks in our main build
> (without unit-test) that contributes to it taking too long.
>
> Also, the way Gradle handles junit categories is designed and coded very
> inefficiently -- if we could change their junit runner to use
> FastClasspathScanner to find all tests containing the targeted junit
> category annotation then that would speed up all of our testing targets
> immensely. Any testing target that forks JVMs runs super slow due to the
> way they handle categories (this effects IntegrationTest, DistributedTest,
> FlakyTest).
>
> On Fri, Sep 15, 2017 at 8:44 AM, Alexander Murmann <amurmann@pivotal.io>
> wrote:
>
> > I fully agree with Udo here. The main build should be for Unit tests. Our
> > "Unit Tests" are already exercising much more of the system than they
> > should. Adding unit tests that not only too much or our current code but
> > also old code is moving us in the wrong direction. Let's keep the tests,
> > but please appropriately mark them as IntegrationTest.
> >
> > On Tue, Sep 12, 2017 at 9:30 AM, Udo Kohlmeyer <ukohlmeyer@pivotal.io>
> > wrote:
> >
> > > My apologies, I might gotten the commit reason incorrect. I just know
> > that
> > > downloading the older product version every time is becoming painful.
> > > Yes, sometimes it is faster than other times, but imo, this is not
> > > something that should be part of the main build path.
> > >
> > > Backwards compat or integration testing should not be running as part
> of
> > > the main build task.
> > >
> > > --Udo
> > >
> > > On Tue, Sep 12, 2017 at 9:05 AM, Nabarun Nag <nnag@apache.org> wrote:
> > >
> > > > As we are working on fixing this issue, some extra parameters may
> help
> > > the
> > > > build to get bit quicker on your machine.
> > > >
> > > > using -xjavadoc -xdoc
> > > > Eg: ./gradlew clean build -Dskip.tests=true -xjavadoc -xdocs
> > > > BUILD SUCCESSFUL
> > > > Total time: 2 mins 2.729 secs
> > > >
> > > >
> > > > Also, I think as Jason mentioned that the slow down is due to full
> > > product
> > > > download for session state tests. LuceneSearchWithRollingUpgradeDUnit
> > > > tests
> > > > were added  in July. Please do correct me if I am wrong.
> > > >
> > > > Regards
> > > > Nabarun
> > > >
> > > >
> > > > On Tue, Sep 12, 2017 at 11:47 AM Alexander Murmann <
> > amurmann@pivotal.io>
> > > > wrote:
> > > >
> > > > > Could we make it so that these tests for now are only run as part
> of
> > > > > pre-checkin till we got this ironed out and then revisit this?
> > > > >
> > > > > On Tue, Sep 12, 2017 at 8:32 AM, Bruce Schuchardt <
> > > > bschuchardt@pivotal.io>
> > > > > wrote:
> > > > >
> > > > > > The geode-old-versions module was originally created to pull
in
> old
> > > > > > version jar files into your gradle cache.  This happened only
> once
> > > and
> > > > > you
> > > > > > were good to go.  I don't think that part should be backed out
as
> > it
> > > > has
> > > > > > minimal impact and is not affecting build time.
> > > > > >
> > > > > > The recent changes for lucene testing seem to be pulling in
full
> > > > > > installations of old versions and these are deleted as part
of
> the
> > > > > "clean"
> > > > > > gradle task.  That's causing them to be downloaded again each
> time
> > > you
> > > > > do a
> > > > > > clean&build.  Dan put changes in place so that the files
aren't
> > > > > downloaded
> > > > > > again if you build without cleaning but clearly more needs to
be
> > done
> > > > in
> > > > > > this area.
> > > > > >
> > > > > >
> > > > > >
> > > > > > On 9/11/17 11:23 AM, Jacob Barrett wrote:
> > > > > >
> > > > > >> Agreed, integration tests should not be part of the build
> process.
> > > > This
> > > > > >> is clearly an integration test.
> > > > > >>
> > > > > >> On Sep 11, 2017, at 11:00 AM, Udo Kohlmeyer <udo@apache.org>
> > wrote:
> > > > > >>>
> > > > > >>> Hi there,
> > > > > >>>
> > > > > >>> With a recent addition to the build scripts, to test
lucene
> > > backwards
> > > > > >>> compatibility, a step was added to download a previous
version
> of
> > > > > GEODE.
> > > > > >>>
> > > > > >>> This is causing longer build times now, which is a real
> > > distraction.
> > > > In
> > > > > >>> cases where one would like to work on a branch, rebase
that on
> > > > develop
> > > > > and
> > > > > >>> merge that, this step becomes a real time hog.
> > > > > >>>
> > > > > >>> I request that we remove this default behavior from
a clean
> build
> > > > until
> > > > > >>> we have a better solution to this issue.
> > > > > >>>
> > > > > >>> I also believe that if anyone wants to add behavior
like this
> > into
> > > > the
> > > > > >>> default build, that it at least is discussed on the
dev list
> > before
> > > > > >>> implementing this.
> > > > > >>>
> > > > > >>> --Udo
> > > > > >>>
> > > > > >>>
> > > > > >
> > > > >
> > > >
> > >
> > >
> > >
> > > --
> > > Kindest Regards
> > > -----------------------------
> > > *Udo Kohlmeyer* | *Snr Solutions Architect* |*Pivotal*
> > > *Mobile:* +61 409-279-160 <+61%20409%20279%20160> |
> ukohlmeyer@pivotal.io
> > > <http://www.gopivotal.com/>
> > > www.pivotal.io
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message