You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Jason Huynh <jh...@pivotal.io> on 2019/10/31 21:04:28 UTC
[vote/discuss]Override stressNewTest for Pull Request #4250?
Greetings,
We have a pull request (https://github.com/apache/geode/pull/4250) that is
running into a problem with stressNewTest. Mostly the tests that are being
run are RollingUpgrade tests that take quite a bit of time to run the full
suite. Because these tests are added/modified, the stressNewTest doesn't
have enough time to complete the run because it runs them N(50) number of
times.
However what has completed is 7400 tests and none of them have failed:
http://files.apachegeode-ci.info/builds/apache-develop-pr/geode-pr-4250/test-results/repeatTest/1572546653/
We would like to get this fix in before branching the next release, but are
unable to due to stressNewTest gating the merge button. I know we have
another thread about overrides etc, and maybe this is a data point, but
this isn't meant to discuss that.
Would everyone be able to agree to allow someone to manually override and
merge this commit in (title of PR and reviews pending)?
Re: [vote/discuss]Override stressNewTest for Pull Request #4250?
Posted by Nabarun Nag <nn...@apache.org>.
Will breaking this PR into parts help. Put the tests in separate PRs?
+1 if it is inconvenient
On Thu, Oct 31, 2019 at 2:52 PM Donal Evans <do...@pivotal.io> wrote:
> +1 to allowing this PR to be merged, although I'd lean strongly toward
> facilitating this by temporarily increasing the timeout on the job to allow
> it to actually pass rather than a manual override of the StressNewTest.
>
> The fact that it's passed over 7000 times without failing is pretty strong
> evidence that it's not a flaky test, which is what StressNewTest is
> supposed to catch, so there doesn't seem to be any risk associated with
> circumventing it in this case, but if there's a feasible solution that
> doesn't involve "cheating" or ignoring the test job, then that would be
> preferable.
>
> - Donal
>
> On Thu, Oct 31, 2019 at 2:04 PM Jason Huynh <jh...@pivotal.io> wrote:
>
> > Greetings,
> >
> > We have a pull request (https://github.com/apache/geode/pull/4250) that
> is
> > running into a problem with stressNewTest. Mostly the tests that are
> being
> > run are RollingUpgrade tests that take quite a bit of time to run the
> full
> > suite. Because these tests are added/modified, the stressNewTest doesn't
> > have enough time to complete the run because it runs them N(50) number of
> > times.
> >
> > However what has completed is 7400 tests and none of them have failed:
> >
> >
> http://files.apachegeode-ci.info/builds/apache-develop-pr/geode-pr-4250/test-results/repeatTest/1572546653/
> >
> > We would like to get this fix in before branching the next release, but
> are
> > unable to due to stressNewTest gating the merge button. I know we have
> > another thread about overrides etc, and maybe this is a data point, but
> > this isn't meant to discuss that.
> >
> > Would everyone be able to agree to allow someone to manually override and
> > merge this commit in (title of PR and reviews pending)?
> >
>
Re: [vote/discuss]Override stressNewTest for Pull Request #4250?
Posted by Xiaojian Zhou <gz...@pivotal.io>.
It finished after 4 hour 51 minutes. It looks like we do need to increase
the timeout for stressNewTest.
On Thu, Oct 31, 2019 at 4:45 PM Darrel Schneider <ds...@pivotal.io>
wrote:
> +1
>
> On Thu, Oct 31, 2019 at 4:16 PM Jinmei Liao <ji...@pivotal.io> wrote:
>
> > +1
> >
> > On Thu, Oct 31, 2019, 3:30 PM Xiaojian Zhou <gz...@pivotal.io> wrote:
> >
> > > I'm curious to see the new stressNew test result too.
> > >
> > > On Thu, Oct 31, 2019 at 3:26 PM Owen Nichols <on...@pivotal.io>
> > wrote:
> > >
> > > > I’ve retriggered StressNew <
> > > >
> > >
> >
> https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/StressNewTestOpenJDK11/builds/4758
> > > >
> > > > with a temporarily-increased timeout of 12 hours so we can see how
> long
> > > it
> > > > would actually take, to have some data point whether to propose a
> > > permanent
> > > > timeout increase or whether breaking up into multiple PRs is should
> be
> > > the
> > > > standard way to get around this.
> > > >
> > > > > On Oct 31, 2019, at 2:52 PM, Donal Evans <do...@pivotal.io>
> wrote:
> > > > >
> > > > > +1 to allowing this PR to be merged, although I'd lean strongly
> > toward
> > > > > facilitating this by temporarily increasing the timeout on the job
> to
> > > > allow
> > > > > it to actually pass rather than a manual override of the
> > StressNewTest.
> > > > >
> > > > > The fact that it's passed over 7000 times without failing is pretty
> > > > strong
> > > > > evidence that it's not a flaky test, which is what StressNewTest is
> > > > > supposed to catch, so there doesn't seem to be any risk associated
> > with
> > > > > circumventing it in this case, but if there's a feasible solution
> > that
> > > > > doesn't involve "cheating" or ignoring the test job, then that
> would
> > be
> > > > > preferable.
> > > > >
> > > > > - Donal
> > > > >
> > > > > On Thu, Oct 31, 2019 at 2:04 PM Jason Huynh <jh...@pivotal.io>
> > wrote:
> > > > >
> > > > >> Greetings,
> > > > >>
> > > > >> We have a pull request (https://github.com/apache/geode/pull/4250
> )
> > > > that is
> > > > >> running into a problem with stressNewTest. Mostly the tests that
> > are
> > > > being
> > > > >> run are RollingUpgrade tests that take quite a bit of time to run
> > the
> > > > full
> > > > >> suite. Because these tests are added/modified, the stressNewTest
> > > > doesn't
> > > > >> have enough time to complete the run because it runs them N(50)
> > number
> > > > of
> > > > >> times.
> > > > >>
> > > > >> However what has completed is 7400 tests and none of them have
> > failed:
> > > > >>
> > > > >>
> > > >
> > >
> >
> http://files.apachegeode-ci.info/builds/apache-develop-pr/geode-pr-4250/test-results/repeatTest/1572546653/
> > > > >>
> > > > >> We would like to get this fix in before branching the next
> release,
> > > but
> > > > are
> > > > >> unable to due to stressNewTest gating the merge button. I know we
> > > have
> > > > >> another thread about overrides etc, and maybe this is a data
> point,
> > > but
> > > > >> this isn't meant to discuss that.
> > > > >>
> > > > >> Would everyone be able to agree to allow someone to manually
> > override
> > > > and
> > > > >> merge this commit in (title of PR and reviews pending)?
> > > > >>
> > > >
> > > >
> > >
> >
>
Re: [vote/discuss]Override stressNewTest for Pull Request #4250?
Posted by Darrel Schneider <ds...@pivotal.io>.
+1
On Thu, Oct 31, 2019 at 4:16 PM Jinmei Liao <ji...@pivotal.io> wrote:
> +1
>
> On Thu, Oct 31, 2019, 3:30 PM Xiaojian Zhou <gz...@pivotal.io> wrote:
>
> > I'm curious to see the new stressNew test result too.
> >
> > On Thu, Oct 31, 2019 at 3:26 PM Owen Nichols <on...@pivotal.io>
> wrote:
> >
> > > I’ve retriggered StressNew <
> > >
> >
> https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/StressNewTestOpenJDK11/builds/4758
> > >
> > > with a temporarily-increased timeout of 12 hours so we can see how long
> > it
> > > would actually take, to have some data point whether to propose a
> > permanent
> > > timeout increase or whether breaking up into multiple PRs is should be
> > the
> > > standard way to get around this.
> > >
> > > > On Oct 31, 2019, at 2:52 PM, Donal Evans <do...@pivotal.io> wrote:
> > > >
> > > > +1 to allowing this PR to be merged, although I'd lean strongly
> toward
> > > > facilitating this by temporarily increasing the timeout on the job to
> > > allow
> > > > it to actually pass rather than a manual override of the
> StressNewTest.
> > > >
> > > > The fact that it's passed over 7000 times without failing is pretty
> > > strong
> > > > evidence that it's not a flaky test, which is what StressNewTest is
> > > > supposed to catch, so there doesn't seem to be any risk associated
> with
> > > > circumventing it in this case, but if there's a feasible solution
> that
> > > > doesn't involve "cheating" or ignoring the test job, then that would
> be
> > > > preferable.
> > > >
> > > > - Donal
> > > >
> > > > On Thu, Oct 31, 2019 at 2:04 PM Jason Huynh <jh...@pivotal.io>
> wrote:
> > > >
> > > >> Greetings,
> > > >>
> > > >> We have a pull request (https://github.com/apache/geode/pull/4250)
> > > that is
> > > >> running into a problem with stressNewTest. Mostly the tests that
> are
> > > being
> > > >> run are RollingUpgrade tests that take quite a bit of time to run
> the
> > > full
> > > >> suite. Because these tests are added/modified, the stressNewTest
> > > doesn't
> > > >> have enough time to complete the run because it runs them N(50)
> number
> > > of
> > > >> times.
> > > >>
> > > >> However what has completed is 7400 tests and none of them have
> failed:
> > > >>
> > > >>
> > >
> >
> http://files.apachegeode-ci.info/builds/apache-develop-pr/geode-pr-4250/test-results/repeatTest/1572546653/
> > > >>
> > > >> We would like to get this fix in before branching the next release,
> > but
> > > are
> > > >> unable to due to stressNewTest gating the merge button. I know we
> > have
> > > >> another thread about overrides etc, and maybe this is a data point,
> > but
> > > >> this isn't meant to discuss that.
> > > >>
> > > >> Would everyone be able to agree to allow someone to manually
> override
> > > and
> > > >> merge this commit in (title of PR and reviews pending)?
> > > >>
> > >
> > >
> >
>
Re: [vote/discuss]Override stressNewTest for Pull Request #4250?
Posted by Jinmei Liao <ji...@pivotal.io>.
+1
On Thu, Oct 31, 2019, 3:30 PM Xiaojian Zhou <gz...@pivotal.io> wrote:
> I'm curious to see the new stressNew test result too.
>
> On Thu, Oct 31, 2019 at 3:26 PM Owen Nichols <on...@pivotal.io> wrote:
>
> > I’ve retriggered StressNew <
> >
> https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/StressNewTestOpenJDK11/builds/4758
> >
> > with a temporarily-increased timeout of 12 hours so we can see how long
> it
> > would actually take, to have some data point whether to propose a
> permanent
> > timeout increase or whether breaking up into multiple PRs is should be
> the
> > standard way to get around this.
> >
> > > On Oct 31, 2019, at 2:52 PM, Donal Evans <do...@pivotal.io> wrote:
> > >
> > > +1 to allowing this PR to be merged, although I'd lean strongly toward
> > > facilitating this by temporarily increasing the timeout on the job to
> > allow
> > > it to actually pass rather than a manual override of the StressNewTest.
> > >
> > > The fact that it's passed over 7000 times without failing is pretty
> > strong
> > > evidence that it's not a flaky test, which is what StressNewTest is
> > > supposed to catch, so there doesn't seem to be any risk associated with
> > > circumventing it in this case, but if there's a feasible solution that
> > > doesn't involve "cheating" or ignoring the test job, then that would be
> > > preferable.
> > >
> > > - Donal
> > >
> > > On Thu, Oct 31, 2019 at 2:04 PM Jason Huynh <jh...@pivotal.io> wrote:
> > >
> > >> Greetings,
> > >>
> > >> We have a pull request (https://github.com/apache/geode/pull/4250)
> > that is
> > >> running into a problem with stressNewTest. Mostly the tests that are
> > being
> > >> run are RollingUpgrade tests that take quite a bit of time to run the
> > full
> > >> suite. Because these tests are added/modified, the stressNewTest
> > doesn't
> > >> have enough time to complete the run because it runs them N(50) number
> > of
> > >> times.
> > >>
> > >> However what has completed is 7400 tests and none of them have failed:
> > >>
> > >>
> >
> http://files.apachegeode-ci.info/builds/apache-develop-pr/geode-pr-4250/test-results/repeatTest/1572546653/
> > >>
> > >> We would like to get this fix in before branching the next release,
> but
> > are
> > >> unable to due to stressNewTest gating the merge button. I know we
> have
> > >> another thread about overrides etc, and maybe this is a data point,
> but
> > >> this isn't meant to discuss that.
> > >>
> > >> Would everyone be able to agree to allow someone to manually override
> > and
> > >> merge this commit in (title of PR and reviews pending)?
> > >>
> >
> >
>
Re: [vote/discuss]Override stressNewTest for Pull Request #4250?
Posted by Xiaojian Zhou <gz...@pivotal.io>.
I'm curious to see the new stressNew test result too.
On Thu, Oct 31, 2019 at 3:26 PM Owen Nichols <on...@pivotal.io> wrote:
> I’ve retriggered StressNew <
> https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/StressNewTestOpenJDK11/builds/4758>
> with a temporarily-increased timeout of 12 hours so we can see how long it
> would actually take, to have some data point whether to propose a permanent
> timeout increase or whether breaking up into multiple PRs is should be the
> standard way to get around this.
>
> > On Oct 31, 2019, at 2:52 PM, Donal Evans <do...@pivotal.io> wrote:
> >
> > +1 to allowing this PR to be merged, although I'd lean strongly toward
> > facilitating this by temporarily increasing the timeout on the job to
> allow
> > it to actually pass rather than a manual override of the StressNewTest.
> >
> > The fact that it's passed over 7000 times without failing is pretty
> strong
> > evidence that it's not a flaky test, which is what StressNewTest is
> > supposed to catch, so there doesn't seem to be any risk associated with
> > circumventing it in this case, but if there's a feasible solution that
> > doesn't involve "cheating" or ignoring the test job, then that would be
> > preferable.
> >
> > - Donal
> >
> > On Thu, Oct 31, 2019 at 2:04 PM Jason Huynh <jh...@pivotal.io> wrote:
> >
> >> Greetings,
> >>
> >> We have a pull request (https://github.com/apache/geode/pull/4250)
> that is
> >> running into a problem with stressNewTest. Mostly the tests that are
> being
> >> run are RollingUpgrade tests that take quite a bit of time to run the
> full
> >> suite. Because these tests are added/modified, the stressNewTest
> doesn't
> >> have enough time to complete the run because it runs them N(50) number
> of
> >> times.
> >>
> >> However what has completed is 7400 tests and none of them have failed:
> >>
> >>
> http://files.apachegeode-ci.info/builds/apache-develop-pr/geode-pr-4250/test-results/repeatTest/1572546653/
> >>
> >> We would like to get this fix in before branching the next release, but
> are
> >> unable to due to stressNewTest gating the merge button. I know we have
> >> another thread about overrides etc, and maybe this is a data point, but
> >> this isn't meant to discuss that.
> >>
> >> Would everyone be able to agree to allow someone to manually override
> and
> >> merge this commit in (title of PR and reviews pending)?
> >>
>
>
Re: [vote/discuss]Override stressNewTest for Pull Request #4250?
Posted by Owen Nichols <on...@pivotal.io>.
I’ve retriggered StressNew <https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/StressNewTestOpenJDK11/builds/4758> with a temporarily-increased timeout of 12 hours so we can see how long it would actually take, to have some data point whether to propose a permanent timeout increase or whether breaking up into multiple PRs is should be the standard way to get around this.
> On Oct 31, 2019, at 2:52 PM, Donal Evans <do...@pivotal.io> wrote:
>
> +1 to allowing this PR to be merged, although I'd lean strongly toward
> facilitating this by temporarily increasing the timeout on the job to allow
> it to actually pass rather than a manual override of the StressNewTest.
>
> The fact that it's passed over 7000 times without failing is pretty strong
> evidence that it's not a flaky test, which is what StressNewTest is
> supposed to catch, so there doesn't seem to be any risk associated with
> circumventing it in this case, but if there's a feasible solution that
> doesn't involve "cheating" or ignoring the test job, then that would be
> preferable.
>
> - Donal
>
> On Thu, Oct 31, 2019 at 2:04 PM Jason Huynh <jh...@pivotal.io> wrote:
>
>> Greetings,
>>
>> We have a pull request (https://github.com/apache/geode/pull/4250) that is
>> running into a problem with stressNewTest. Mostly the tests that are being
>> run are RollingUpgrade tests that take quite a bit of time to run the full
>> suite. Because these tests are added/modified, the stressNewTest doesn't
>> have enough time to complete the run because it runs them N(50) number of
>> times.
>>
>> However what has completed is 7400 tests and none of them have failed:
>>
>> http://files.apachegeode-ci.info/builds/apache-develop-pr/geode-pr-4250/test-results/repeatTest/1572546653/
>>
>> We would like to get this fix in before branching the next release, but are
>> unable to due to stressNewTest gating the merge button. I know we have
>> another thread about overrides etc, and maybe this is a data point, but
>> this isn't meant to discuss that.
>>
>> Would everyone be able to agree to allow someone to manually override and
>> merge this commit in (title of PR and reviews pending)?
>>
Re: [vote/discuss]Override stressNewTest for Pull Request #4250?
Posted by Donal Evans <do...@pivotal.io>.
+1 to allowing this PR to be merged, although I'd lean strongly toward
facilitating this by temporarily increasing the timeout on the job to allow
it to actually pass rather than a manual override of the StressNewTest.
The fact that it's passed over 7000 times without failing is pretty strong
evidence that it's not a flaky test, which is what StressNewTest is
supposed to catch, so there doesn't seem to be any risk associated with
circumventing it in this case, but if there's a feasible solution that
doesn't involve "cheating" or ignoring the test job, then that would be
preferable.
- Donal
On Thu, Oct 31, 2019 at 2:04 PM Jason Huynh <jh...@pivotal.io> wrote:
> Greetings,
>
> We have a pull request (https://github.com/apache/geode/pull/4250) that is
> running into a problem with stressNewTest. Mostly the tests that are being
> run are RollingUpgrade tests that take quite a bit of time to run the full
> suite. Because these tests are added/modified, the stressNewTest doesn't
> have enough time to complete the run because it runs them N(50) number of
> times.
>
> However what has completed is 7400 tests and none of them have failed:
>
> http://files.apachegeode-ci.info/builds/apache-develop-pr/geode-pr-4250/test-results/repeatTest/1572546653/
>
> We would like to get this fix in before branching the next release, but are
> unable to due to stressNewTest gating the merge button. I know we have
> another thread about overrides etc, and maybe this is a data point, but
> this isn't meant to discuss that.
>
> Would everyone be able to agree to allow someone to manually override and
> merge this commit in (title of PR and reviews pending)?
>