@Kenn,
Yes it is through the Elasticsearch test framework that you reference that I
discovered that direct runner counter
metrics thread leaked. I opened this ticket
https://issues.apache.org/jira/browse/BEAM-3119 at the time.
But it seemed overkill to ship ES test framework in Romain's PR unit tests just
to detect thread leaks. So Romain did a
manual detection process.
Etienne
Le mercredi 18 avril 2018 à 20:50 +0000, Kenneth Knowles a écrit :
> For this particular test, is it failing because a different test class leaked
> threads? Is there a way to make
> something like a testing base class with @Before and @After actions that
> would catch the leak closer to where it
> happened? A quick search found this:
> https://github.com/elastic/elasticsearch-mapper-attachments/issues/88 but if
> that
> doesn't apply maybe something else does. The very best thing is probably to
> use a dynamic analysis since it is not
> very practical to unit test for leaks (thread, memory, file handle, whatever)
> and other global correctness criteria.
>
> Kenn
>
> On Wed, Apr 18, 2018 at 10:13 AM Romain Manni-Bucau <rmannibu...@gmail.com>
> wrote:
> >
> >
> > Le 18 avr. 2018 18:53, "Scott Wegner" <sweg...@google.com> a écrit :
> > I wanted to provide some additional information about this issue and Gradle
> > test configuration.
> >
> > For context, BEAM-4088 [1] refers to a new test on PR/4965 [2] which
> > verifies that no threads are being leaked.
> > Under Maven the test passes, but under Gradle it reports leaked threads [3].
> >
> > I'm not familiar with this test's logic, but presumably some differences in
> > test isolation are causing the failures.
> > In Gradle, test classes are executed in parallel in forked VMs, and those
> > VMs are reused between test classes. This
> > configuration optimizes for execution time, but can break if tests are not
> > well isolated.
> >
> > When a test fails in this configuration, the first step should be to
> > understand why it fails and see if the test can
> > be implemented with better isolation. Barring that, we can update the
> > execution parameters at the project level for
> > stronger sandboxing at the cost of longer execution time. In Gradle, this
> > is done by setting maxParallelForks [4]
> > and forkEvery [5] options on the Test task.
> >
> > I dont know, first of all no hardcoded value must be in any build file
> > until required. A required case can be "fork
> > and dont reuse the forks" cause this IO is known as leaking.
> >
> > MaxFork and forEvery are about forks.
> > The test works if it runs alone in a fork until another test execution
> > leaked and still runs.
> >
> > Killing the daemon can help (and is a good practise on the CI), maybe worth
> > a try.
> >
> > We can also fork a thread for rhis particular test bit it just hides a
> > nasty bug so probably not worth it.
> >
> >
> > [1] https://issues.apache.org/jira/browse/BEAM-4088
> > [2] https://github.com/apache/beam/pull/4965
> > [3]
> > https://scans.gradle.com/s/grha56432j3t2/tests/jqhvlvf72f7pg-ipde5etqqejoa?openStackTraces=WzBd
> >
> > [4]
> > https://docs.gradle.org/current/dsl/org.gradle.api.tasks.testing.Test.html#org.gradle.api.tasks.testing.Test:max
> > ParallelForks
> > [5]
> > https://docs.gradle.org/current/dsl/org.gradle.api.tasks.testing.Test.html#org.gradle.api.tasks.testing.Test:for
> > kEvery
> >
> > On Thu, Apr 12, 2018 at 10:03 AM Romain Manni-Bucau <rmannibu...@gmail.com>
> > wrote:
> > > Seems so but is not. Most of beam tests assume they are alone in their vm
> > > when executing for a bunch of reason, if
> > > not the case a lot of side effects can happen (backend state, local cache
> > > drop, ...., uncontrolled resources and
> > > failure due to the GBKTest which creates 100M keys etc...) so you have to
> > > have one test per vm at any time. If
> > > this assumption is true my test will pass, if it is false gradle setup is
> > > wrong.
> > >
> > > Le 12 avr. 2018 18:40, "Kenneth Knowles" <k...@google.com> a écrit :
> > > > It seems that the test probably depends on some details of Maven or our
> > > > Maven configuration. If so, that's a
> > > > problem with the test. It should be able to succeed in any build
> > > > system, or as a standalone JUnit main built
> > > > from the suite, etc.
> > > >
> > > > Kenn
> > > >
> > > > On Thu, Apr 12, 2018 at 9:27 AM Romain Manni-Bucau
> > > > <rmannibu...@gmail.com> wrote:
> > > > > on maven it is quite reliable, ran it > 10 times without any failure.
> > > > >
> > > > > I suspect (but didnt check by lack of time) gradle parallelism is
> > > > > different somehow and can lead to some flackyness here.
> > > > >
> > > > > Romain Manni-Bucau
> > > > > @rmannibucau | Blog | Old Blog | Github | LinkedIn | Book
> > > > >
> > > > >
> > > > > 2018-04-12 18:20 GMT+02:00 Scott Wegner <sweg...@google.com>:
> > > > > > It looks like the precommit failure [1] is for a new test that was
> > > > > > added.
> > > > > > Have you debugged the test to ensure it's not flaky?
> > > > > >
> > > > > > [1]
> > > > > > https://builds.apache.org/job/beam_PreCommit_Java_GradleBuild/4059/testReport/junit/org.apache.beam.runners.
> > > > > direct/ExecutorServiceParallelExecutorTest/ensureMetricsThreadDoesntLeak/
> > > > > >
> > > > > > On Thu, Apr 12, 2018 at 5:06 AM Romain Manni-Bucau
> > > > > > <rmannibu...@gmail.com>
> > > > > > wrote:
> > > > > >>
> > > > > >> Hi guys,
> > > > > >>
> > > > > >> did the gradle track changed the way test execution was done?
> > > > > >>
> > > > > >> This PR https://github.com/apache/beam/pull/4965 works very well
> > > > > >> with
> > > > > >> maven and sometimes doesn't pass with gradle. Think we should keep
> > > > > >> the
> > > > > >> previous setup which was globally reliable (I'm not speaking of
> > > > > >> tests
> > > > > >> which are not but of the setup).
> > > > > >>
> > > > > >> Any inputs or in progress todo i missed?
> > > > > >>
> > > > > >> Romain Manni-Bucau
> > > > > >> @rmannibucau | Blog | Old Blog | Github | LinkedIn | Book
> > > > > >
> > > > > > --
> > > > > >
> > > > > >
> > > > > > Got feedback? http://go/swegner-feedback
> > > > >
> > --
> >
> >
> > Got feedback? http://go/swegner-feedback
> >
> >
