cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Kjellman (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-13216) testall failure in org.apache.cassandra.net.MessagingServiceTest.testDroppedMessages
Date Mon, 08 May 2017 21:55:04 GMT

    [ 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: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org


Mime
View raw message