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