You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by Andor Molnar <an...@apache.org> on 2019/07/22 08:47:32 UTC
New flaky: testLearnerMetricsTest
Hi team,
New test testLearnerMetricsTest() added by the following commit failed 2 times on master with the same error:
junit.framework.AssertionFailedError: expected:<10> but was:<9>
at org.apache.zookeeper.server.quorum.LearnerMetricsTest.testLearnerMetricsTest(LearnerMetricsTest.java:88)
at org.apache.zookeeper.JUnit4ZKTestRunner$LoggedInvokeMethod.evaluate(JUnit4ZKTestRunner.java:80)
https://github.com/apache/zookeeper/pull/856
Submitted by: jhuan31
Maybe we should just blame the Thread.sleep(200) in line:77 and replace it with some clever logic.
Please take a look.
Regards,
Andor
Re: New flaky: testLearnerMetricsTest
Posted by Andor Molnar <an...@apache.org>.
Jira created.
https://issues.apache.org/jira/browse/ZOOKEEPER-3470
Andor
> On 2019. Jul 24., at 8:17, Enrico Olivelli <eo...@gmail.com> wrote:
>
> Il mar 23 lug 2019, 22:30 Patrick Hunt <ph...@apache.org> ha scritto:
>
>> Yea, thread sleep (without some sort of retry/timeout logic) is an
>> antipattern. Committers - please be on the lookout for such things and flag
>> them during review as they are a significant cause of flakeys. In a heavily
>> loaded or virtualized environment even seconds can be insufficient.
>>
>> QuorumTest line 320 or so is a better example. In many places we use
>> countdownlatches instead...
>>
>> I see someone attempted to add "waitFor" but it never caught on? Might be
>> better approach.
>>
>
> It is really a better practice.
>
> Enrico
>
>
>> zookeeper-server/src/test/java/org/apache/zookeeper/ZKTestCase.java
>> 95: public void waitFor(String msg, WaitForCondition condition, int
>> timeout)
>>
>> Patrick
>>
>> On Mon, Jul 22, 2019 at 1:47 AM Andor Molnar <an...@apache.org> wrote:
>>
>>> Hi team,
>>>
>>> New test testLearnerMetricsTest() added by the following commit failed 2
>>> times on master with the same error:
>>>
>>> junit.framework.AssertionFailedError: expected:<10> but was:<9>
>>> at
>>>
>> org.apache.zookeeper.server.quorum.LearnerMetricsTest.testLearnerMetricsTest(LearnerMetricsTest.java:88)
>>> at
>>>
>> org.apache.zookeeper.JUnit4ZKTestRunner$LoggedInvokeMethod.evaluate(JUnit4ZKTestRunner.java:80)
>>>
>>> https://github.com/apache/zookeeper/pull/856
>>> Submitted by: jhuan31
>>>
>>> Maybe we should just blame the Thread.sleep(200) in line:77 and replace
>> it
>>> with some clever logic.
>>> Please take a look.
>>>
>>> Regards,
>>> Andor
>>>
>>>
>>>
>>
Re: New flaky: testLearnerMetricsTest
Posted by Enrico Olivelli <eo...@gmail.com>.
Il mar 23 lug 2019, 22:30 Patrick Hunt <ph...@apache.org> ha scritto:
> Yea, thread sleep (without some sort of retry/timeout logic) is an
> antipattern. Committers - please be on the lookout for such things and flag
> them during review as they are a significant cause of flakeys. In a heavily
> loaded or virtualized environment even seconds can be insufficient.
>
> QuorumTest line 320 or so is a better example. In many places we use
> countdownlatches instead...
>
> I see someone attempted to add "waitFor" but it never caught on? Might be
> better approach.
>
It is really a better practice.
Enrico
> zookeeper-server/src/test/java/org/apache/zookeeper/ZKTestCase.java
> 95: public void waitFor(String msg, WaitForCondition condition, int
> timeout)
>
> Patrick
>
> On Mon, Jul 22, 2019 at 1:47 AM Andor Molnar <an...@apache.org> wrote:
>
> > Hi team,
> >
> > New test testLearnerMetricsTest() added by the following commit failed 2
> > times on master with the same error:
> >
> > junit.framework.AssertionFailedError: expected:<10> but was:<9>
> > at
> >
> org.apache.zookeeper.server.quorum.LearnerMetricsTest.testLearnerMetricsTest(LearnerMetricsTest.java:88)
> > at
> >
> org.apache.zookeeper.JUnit4ZKTestRunner$LoggedInvokeMethod.evaluate(JUnit4ZKTestRunner.java:80)
> >
> > https://github.com/apache/zookeeper/pull/856
> > Submitted by: jhuan31
> >
> > Maybe we should just blame the Thread.sleep(200) in line:77 and replace
> it
> > with some clever logic.
> > Please take a look.
> >
> > Regards,
> > Andor
> >
> >
> >
>
Re: New flaky: testLearnerMetricsTest
Posted by Patrick Hunt <ph...@apache.org>.
Yea, thread sleep (without some sort of retry/timeout logic) is an
antipattern. Committers - please be on the lookout for such things and flag
them during review as they are a significant cause of flakeys. In a heavily
loaded or virtualized environment even seconds can be insufficient.
QuorumTest line 320 or so is a better example. In many places we use
countdownlatches instead...
I see someone attempted to add "waitFor" but it never caught on? Might be
better approach.
zookeeper-server/src/test/java/org/apache/zookeeper/ZKTestCase.java
95: public void waitFor(String msg, WaitForCondition condition, int
timeout)
Patrick
On Mon, Jul 22, 2019 at 1:47 AM Andor Molnar <an...@apache.org> wrote:
> Hi team,
>
> New test testLearnerMetricsTest() added by the following commit failed 2
> times on master with the same error:
>
> junit.framework.AssertionFailedError: expected:<10> but was:<9>
> at
> org.apache.zookeeper.server.quorum.LearnerMetricsTest.testLearnerMetricsTest(LearnerMetricsTest.java:88)
> at
> org.apache.zookeeper.JUnit4ZKTestRunner$LoggedInvokeMethod.evaluate(JUnit4ZKTestRunner.java:80)
>
> https://github.com/apache/zookeeper/pull/856
> Submitted by: jhuan31
>
> Maybe we should just blame the Thread.sleep(200) in line:77 and replace it
> with some clever logic.
> Please take a look.
>
> Regards,
> Andor
>
>
>