Avro
  1. Avro
  2. AVRO-994

TestFileSpanStorage.testTonsOfSpans() fails on my slow VM

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.6.0, 1.6.1
    • Fix Version/s: 1.6.2, 1.7.0
    • Component/s: java
    • Labels:
      None

      Description

      Tests run: 4, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 15.554 sec <<< FAILURE!
      testTonsOfSpans(org.apache.avro.ipc.trace.TestFileSpanStorage)  Time elapsed: 3.853 sec  <<< FAILURE!
      java.lang.AssertionError: expected:<50000> but was:<42356>
              at org.junit.Assert.fail(Assert.java:93)
              at org.junit.Assert.failNotEquals(Assert.java:647)
              at org.junit.Assert.assertEquals(Assert.java:128)
              at org.junit.Assert.assertEquals(Assert.java:472)
              at org.junit.Assert.assertEquals(Assert.java:456)
              at org.apache.avro.ipc.trace.TestFileSpanStorage.testTonsOfSpans(TestFileSpanStorage.java:70)
      

      The issue seems to be the Thread.sleep(2000) on line 66. Doubling this to 4000ms causes the test to pass. In general it might be better to make this sleep event-based rather than using a fixed sleep time. If that isn't possible, then maybe using some sort of a retry loop would work.

      1. AVRO-994.patch
        0.7 kB
        James Baldassari

        Activity

        Hide
        James Baldassari added a comment -

        Here's a patch that uses the retry approach because it doesn't look like there is any event-based interface to hook into.

        Show
        James Baldassari added a comment - Here's a patch that uses the retry approach because it doesn't look like there is any event-based interface to hook into.
        Hide
        Doug Cutting added a comment -

        +1 That's a better way to fix such things, rather than just increasing the sleep time as we have before. Thanks!

        Show
        Doug Cutting added a comment - +1 That's a better way to fix such things, rather than just increasing the sleep time as we have before. Thanks!
        Hide
        Scott Carey added a comment -

        +1

        Show
        Scott Carey added a comment - +1
        Hide
        James Baldassari added a comment -

        I committed this. First commit, so please let me know if there are any issues with the commit log or CHANGES.TXT. Thanks.

        Show
        James Baldassari added a comment - I committed this. First commit, so please let me know if there are any issues with the commit log or CHANGES.TXT. Thanks.
        Hide
        Doug Cutting added a comment -

        Two small things:

        1. Please put the name(s) of the patch author(s) and committer in CHANGES.txt.

        2. Since 1.6.2 will be released before 1.7.0, there's no need to note this as fixed in 1.7.0.

        Other than that, great job and thanks!

        Show
        Doug Cutting added a comment - Two small things: 1. Please put the name(s) of the patch author(s) and committer in CHANGES.txt. 2. Since 1.6.2 will be released before 1.7.0, there's no need to note this as fixed in 1.7.0. Other than that, great job and thanks!
        Hide
        James Baldassari added a comment -

        Thanks for the feedback. I've updated CHANGES.txt.

        Show
        James Baldassari added a comment - Thanks for the feedback. I've updated CHANGES.txt.

          People

          • Assignee:
            James Baldassari
            Reporter:
            James Baldassari
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development