[
https://issues.apache.org/jira/browse/CASSANDRA-13216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16001619#comment-16001619
]
Michael Kjellman commented on CASSANDRA-13216:
----------------------------------------------
With this change, the "mocked" Clock in MessagingServiceTest will always return
0 for getTick()
{code}
private static long time = System.currentTimeMillis();
private static Clock clock = new Clock()
{
public long getTick()
{
return 0;
}
public long getTime()
{
return time;
}
};
{code}
But if we look at the implementation of time() in metrics-core Timer, it does
the following:
{code}
/**
* Times and records the duration of event.
*
* @param event a {@link Callable} whose {@link Callable#call()} method
implements a process
* whose duration should be timed
* @param <T> the type of the value returned by {@code event}
* @return the value returned by {@code event}
* @throws Exception if {@code event} throws an {@link Exception}
*/
public <T> T time(Callable<T> event) throws Exception {
final long startTime = clock.getTick();
try {
return event.call();
} finally {
update(clock.getTick() - startTime);
}
}
{code}
So, from my understanding this means we will always just do 0-0 for the
update() call on the Timer... right?
However, I don't think any of this matters in retospect. Took a big step back
and looked over the actual unit test and what this thing is testing with
[~jjirsa] and [~jasobrown] and all 3 of us think this magic number a bit
questionable.
If we look at the original patch that added these magic numbers in the first
place for CASSANDRA-10580
(https://github.com/pcmanus/cassandra/commit/c9ef25fd81501005b6484baf064081efc557f3f4)
there is nothing in the ticket or test or commit that justifies testing for
these magic numbers and it looks like this is just going to be dependent on how
fast your system can iterate thru the logic 5000 times.
So: I'd like to propose that we throw away the 2nd assert in this test. The
first and last are good (counting the number that we expect to get) but doing a
literal string compare on the entire log message is kinda unhelpful. Instead,
we should throw a regex here on the log message, parse out the times and just
check that they are > 0. Thoughts?
> testall failure in
> org.apache.cassandra.net.MessagingServiceTest.testDroppedMessages
> ------------------------------------------------------------------------------------
>
> Key: CASSANDRA-13216
> URL: https://issues.apache.org/jira/browse/CASSANDRA-13216
> Project: Cassandra
> Issue Type: Bug
> Components: Testing
> Reporter: Sean McCarthy
> Assignee: Alex Petrov
> Labels: test-failure, testall
> Fix For: 3.0.x, 3.11.x, 4.x
>
> Attachments: TEST-org.apache.cassandra.net.MessagingServiceTest.log,
> TEST-org.apache.cassandra.net.MessagingServiceTest.log
>
>
> example failure:
> http://cassci.datastax.com/job/cassandra-3.11_testall/81/testReport/org.apache.cassandra.net/MessagingServiceTest/testDroppedMessages
> {code}
> Error Message
> expected:<... dropped latency: 27[30 ms and Mean cross-node dropped latency:
> 2731] ms> but was:<... dropped latency: 27[28 ms and Mean cross-node dropped
> latency: 2730] ms>
> {code}{code}
> Stacktrace
> junit.framework.AssertionFailedError: expected:<... dropped latency: 27[30 ms
> and Mean cross-node dropped latency: 2731] ms> but was:<... dropped latency:
> 27[28 ms and Mean cross-node dropped latency: 2730] ms>
> at
> org.apache.cassandra.net.MessagingServiceTest.testDroppedMessages(MessagingServiceTest.java:83)
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
