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 <[email protected]> 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
