You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@ratis.apache.org by Ben Horowitz <be...@woozlesaurus.com> on 2022/07/08 22:25:18 UTC
Test failure in TestLeaderElectionMetrics.testOnLeaderElectionCompletion
Hi,
When running unit tests I encountered the following failure:
java.lang.AssertionError: leaderElectionLatency = 0
at org.junit.Assert.fail(Assert.java:89)
at org.junit.Assert.assertTrue(Assert.java:42)
at org.apache.ratis.server.metrics.TestLeaderElectionMetrics.testOnLeaderElectionCompletion(TestLeaderElectionMetrics.java:61)
It seems like the assertion
assertTrue("leaderElectionLatency = " + leaderElectionLatency, leaderElectionLatency > 0L);
is too strict?
Ben
Re: Test failure in TestLeaderElectionMetrics.testOnLeaderElectionCompletion
Posted by Tsz Wo Sze <sz...@gmail.com>.
Hi Ben,
Thanks for digging out the problem! I saw that you had filed
https://issues.apache.org/jira/browse/RATIS-1621 . Let's continue the
discussion there.
Tsz-Wo
On Mon, Jul 11, 2022 at 9:22 AM Ben Horowitz <be...@woozlesaurus.com> wrote:
> Hi Tsz-Wo,
>
> It looks like the intention of the test is to ensure that after
> leaderElectionMetrics.onNewLeaderElectionCompletion()
> is called, LeaderElectionMetrics.lastElectionTime is updated such that the
> latency returned by the gauge is reflective of the last election. One thing
> I am not sure about is the meaning of the latency returned by the gauge --
> does the latency represent the duration between the start of the process
> and the last leader election?
>
> The gauge will return -1 if LeaderElectionMetrics.lastElectionTime not
> updated during leaderElectionMetrics.onNewLeaderElectionCompletion(). In
> this case, LeaderElectionMetrics.lastElectionTime == null, and the gauge
> uses the value of -1L from its definition:
>
> Optional.ofNullable(lastElectionTime).map(Timestamp::elapsedTimeMs).orElse(-1L)))
>
> So I think a reasonable approach would be to change the assertion to
> assertTrue("leaderElectionLatency = " + leaderElectionLatency,
> leaderElectionLatency >= 0L)
>
> This would catch any regression where
> leaderElectionMetrics.onNewLeaderElectionCompletion() does not update
> LeaderElectionMetrics.lastElectionTime.
>
> Ben
>
> On Fri, Jul 8, 2022, at 8:10 PM, Tsz Wo Sze wrote:
> > Hi Ben,
> >
> > Thanks for checking. The test may be too strict. It fails
> occasionally. Do you have any suggestions for fixing it? Please feel free
> to file a JIRA.
> >
> > Tsz-Wo
> >
> > On Fri, Jul 8, 2022 at 3:25 PM Ben Horowitz <be...@woozlesaurus.com>
> wrote:
> >> Hi,
> >>
> >> When running unit tests I encountered the following failure:
> >>
> >> java.lang.AssertionError: leaderElectionLatency = 0
> >>
> >> at org.junit.Assert.fail(Assert.java:89)
> >> at org.junit.Assert.assertTrue(Assert.java:42)
> >> at
> org.apache.ratis.server.metrics.TestLeaderElectionMetrics.testOnLeaderElectionCompletion(TestLeaderElectionMetrics.java:61)
> >>
> >> It seems like the assertion
> >> assertTrue("leaderElectionLatency = " + leaderElectionLatency,
> leaderElectionLatency > 0L);
> >> is too strict?
> >>
> >> Ben
>
Re: Test failure in TestLeaderElectionMetrics.testOnLeaderElectionCompletion
Posted by Ben Horowitz <be...@woozlesaurus.com>.
Hi Tsz-Wo,
It looks like the intention of the test is to ensure that after
leaderElectionMetrics.onNewLeaderElectionCompletion()
is called, LeaderElectionMetrics.lastElectionTime is updated such that the latency returned by the gauge is reflective of the last election. One thing I am not sure about is the meaning of the latency returned by the gauge -- does the latency represent the duration between the start of the process and the last leader election?
The gauge will return -1 if LeaderElectionMetrics.lastElectionTime not updated during leaderElectionMetrics.onNewLeaderElectionCompletion(). In this case, LeaderElectionMetrics.lastElectionTime == null, and the gauge uses the value of -1L from its definition:
Optional.ofNullable(lastElectionTime).map(Timestamp::elapsedTimeMs).orElse(-1L)))
So I think a reasonable approach would be to change the assertion to
assertTrue("leaderElectionLatency = " + leaderElectionLatency, leaderElectionLatency >= 0L)
This would catch any regression where leaderElectionMetrics.onNewLeaderElectionCompletion() does not update LeaderElectionMetrics.lastElectionTime.
Ben
On Fri, Jul 8, 2022, at 8:10 PM, Tsz Wo Sze wrote:
> Hi Ben,
>
> Thanks for checking. The test may be too strict. It fails occasionally. Do you have any suggestions for fixing it? Please feel free to file a JIRA.
>
> Tsz-Wo
>
> On Fri, Jul 8, 2022 at 3:25 PM Ben Horowitz <be...@woozlesaurus.com> wrote:
>> Hi,
>>
>> When running unit tests I encountered the following failure:
>>
>> java.lang.AssertionError: leaderElectionLatency = 0
>>
>> at org.junit.Assert.fail(Assert.java:89)
>> at org.junit.Assert.assertTrue(Assert.java:42)
>> at org.apache.ratis.server.metrics.TestLeaderElectionMetrics.testOnLeaderElectionCompletion(TestLeaderElectionMetrics.java:61)
>>
>> It seems like the assertion
>> assertTrue("leaderElectionLatency = " + leaderElectionLatency, leaderElectionLatency > 0L);
>> is too strict?
>>
>> Ben
Re: Test failure in TestLeaderElectionMetrics.testOnLeaderElectionCompletion
Posted by Tsz Wo Sze <sz...@gmail.com>.
Hi Ben,
Thanks for checking. The test may be too strict. It fails occasionally.
Do you have any suggestions for fixing it? Please feel free to file a JIRA.
Tsz-Wo
On Fri, Jul 8, 2022 at 3:25 PM Ben Horowitz <be...@woozlesaurus.com> wrote:
> Hi,
>
> When running unit tests I encountered the following failure:
>
> java.lang.AssertionError: leaderElectionLatency = 0
>
> at org.junit.Assert.fail(Assert.java:89)
> at org.junit.Assert.assertTrue(Assert.java:42)
> at
> org.apache.ratis.server.metrics.TestLeaderElectionMetrics.testOnLeaderElectionCompletion(TestLeaderElectionMetrics.java:61)
>
> It seems like the assertion
> assertTrue("leaderElectionLatency = " + leaderElectionLatency,
> leaderElectionLatency > 0L);
> is too strict?
>
> Ben
>