Mailing-List: contact commits-help@cassandra.apache.org; run by ezmlm
Precedence: bulk
Date: Mon, 8 May 2017 21:55:04 +0000 (UTC)
From: "Michael Kjellman (JIRA)" <jira@apache.org>
To: commits@cassandra.apache.org
Message-ID: <JIRA.13042588.1486997503000.157265.1494280504440@Atlassian.JIRA>
In-Reply-To: <JIRA.13042588.1486997503000@Atlassian.JIRA>
References: <JIRA.13042588.1486997503000@Atlassian.JIRA> <JIRA.13042588.1486997503520@jira-lw-us.apache.org>
Subject: [jira] [Commented] (CASSANDRA-13216) testall failure in
 org.apache.cassandra.net.MessagingServiceTest.testDroppedMessages
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
archived-at: Mon, 08 May 2017 21:55:11 -0000


    [ 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