Qpid
  1. Qpid
  2. QPID-3459

Test org.apache.qpid.management.jmx.MessageStatisticsDeliveryTest sporadically fails on 0.10 profiles

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.6, 0.7, 0.8, 0.9, 0.10, 0.11, 0.12
    • Fix Version/s: 0.13
    • Component/s: Java Broker
    • Labels:
      None

      Description

      The stack trace from failed test is provided below:

      Message 3 was not received
      junit.framework.AssertionFailedError: Message 3 was not received
      at org.apache.qpid.management.jmx.MessageStatisticsDeliveryTest.receiveUsing(MessageStatisticsDeliveryTest.java:107)
      at org.apache.qpid.management.jmx.MessageStatisticsDeliveryTest.testDeliveryAndReceiptStatistics(MessageStatisticsDeliveryTest.java:72)
      at org.apache.qpid.test.utils.QpidBrokerTestCase.runBare(QpidBrokerTestCase.java:243)
      at org.apache.qpid.test.utils.QpidTestCase.run(QpidTestCase.java:125)

        Activity

        Hide
        Robbie Gemmell added a comment -

        Patch applied.

        Show
        Robbie Gemmell added a comment - Patch applied.
        Hide
        Alex Rudyy added a comment -

        Robbie, could you please review an updated version of patch?

        The patch changes the order of operations:
        the counter is incremented before sending the message over the wire.

        Show
        Alex Rudyy added a comment - Robbie, could you please review an updated version of patch? The patch changes the order of operations: the counter is incremented before sending the message over the wire.
        Hide
        Andrew Kennedy added a comment -

        Agree with Robbie here.

        FYI, when implementing, I deliberated between full-on synchronisation and using atomic counters, eventually settling for the synchronisation as seen. In testing there was no performance difference between the two mechanisms at any statistically significant level. Adding more synchronisation would cause serialisation of message processing, since the statistics generation happens on the delivery path, which is inadvisable.

        Show
        Andrew Kennedy added a comment - Agree with Robbie here. FYI, when implementing, I deliberated between full-on synchronisation and using atomic counters, eventually settling for the synchronisation as seen. In testing there was no performance difference between the two mechanisms at any statistically significant level. Adding more synchronisation would cause serialisation of message processing, since the statistics generation happens on the delivery path, which is inadvisable.
        Hide
        Robbie Gemmell added a comment -

        I dont think the changes in the patch will resolve the underlying test failure.

        From the test we know that the 5th message has been recieved, and yet the statistcs counter still reports 4, meaning it has not yet been incremented. Synchronizing just those methods should have no effect in that scenario, as the value is retrieved via JMX and so involvs another roundtrip to the broker after the message has been received at the client. Given this, and that the occurences of this failure have only been seen on an old dual core machine, I think its likely that thread scheduling is occurring such that the delivering thread has yielded before incrementing the counter, meaning this synchronization wont help.

        Adding additional synchronization on the statistics object around the message delivery and increment would also be required I believe, which isnt going to be nice for performance (and furthermore, the statistics are optional and off by default).An alternative might be to increment the counter immediatley prior to the delivery attempt, although this wasn't done previously since any exceptions thrown during delivery would make the value invalid, and reverting the increment if an exception occurs is not as simple as it sounds since it means manipulating the statistics which may have already moved on to a new reporting interval. Ick to all of the above.

        Show
        Robbie Gemmell added a comment - I dont think the changes in the patch will resolve the underlying test failure. From the test we know that the 5th message has been recieved, and yet the statistcs counter still reports 4, meaning it has not yet been incremented. Synchronizing just those methods should have no effect in that scenario, as the value is retrieved via JMX and so involvs another roundtrip to the broker after the message has been received at the client. Given this, and that the occurences of this failure have only been seen on an old dual core machine, I think its likely that thread scheduling is occurring such that the delivering thread has yielded before incrementing the counter, meaning this synchronization wont help. Adding additional synchronization on the statistics object around the message delivery and increment would also be required I believe, which isnt going to be nice for performance (and furthermore, the statistics are optional and off by default).An alternative might be to increment the counter immediatley prior to the delivery attempt, although this wasn't done previously since any exceptions thrown during delivery would make the value invalid, and reverting the increment if an exception occurs is not as simple as it sounds since it means manipulating the statistics which may have already moved on to a new reporting interval. Ick to all of the above.
        Hide
        Alex Rudyy added a comment -

        Also, sometimes this test fails with the following stack trace

        Incorrect vhost delivery total expected:<5> but was:<4>
        junit.framework.AssertionFailedError: Incorrect vhost delivery total expected:<5> but was:<4>
        at org.apache.qpid.management.jmx.MessageStatisticsDeliveryTest.testDeliveryAndReceiptStatistics(MessageStatisticsDeliveryTest.java:91)
        at org.apache.qpid.test.utils.QpidBrokerTestCase.runBare(QpidBrokerTestCase.java:238)
        at org.apache.qpid.test.utils.QpidTestCase.run(QpidTestCase.java:132)

        Show
        Alex Rudyy added a comment - Also, sometimes this test fails with the following stack trace Incorrect vhost delivery total expected:<5> but was:<4> junit.framework.AssertionFailedError: Incorrect vhost delivery total expected:<5> but was:<4> at org.apache.qpid.management.jmx.MessageStatisticsDeliveryTest.testDeliveryAndReceiptStatistics(MessageStatisticsDeliveryTest.java:91) at org.apache.qpid.test.utils.QpidBrokerTestCase.runBare(QpidBrokerTestCase.java:238) at org.apache.qpid.test.utils.QpidTestCase.run(QpidTestCase.java:132)

          People

          • Assignee:
            Robbie Gemmell
            Reporter:
            Alex Rudyy
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development