Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.23.0
    • Fix Version/s: 0.23.0
    • Component/s: metrics
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Tags:
      metrics, unit test

      Description

      The TestSinkQueue is racy (Thread.yield is not enough to guarantee other intended thread getting run), though it's the first time (from HADOOP-7289) I saw it manifested here.

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk #691 (See https://builds.apache.org/hudson/job/Hadoop-Common-trunk/691/)
          HADOOP-7292. Fix racy test case TestSinkQueue. Contributed by Luke Lu.

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk #691 (See https://builds.apache.org/hudson/job/Hadoop-Common-trunk/691/ ) HADOOP-7292 . Fix racy test case TestSinkQueue. Contributed by Luke Lu.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #604 (See https://builds.apache.org/hudson/job/Hadoop-Common-trunk-Commit/604/)
          HADOOP-7292. Fix racy test case TestSinkQueue. Contributed by Luke Lu.

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #604 (See https://builds.apache.org/hudson/job/Hadoop-Common-trunk-Commit/604/ ) HADOOP-7292 . Fix racy test case TestSinkQueue. Contributed by Luke Lu.
          Hide
          Todd Lipcon added a comment -

          Committed to trunk.

          Show
          Todd Lipcon added a comment - Committed to trunk.
          Hide
          Todd Lipcon added a comment -

          +1. Will commit to trunk momentarily (manually ran the one changed test case)

          Show
          Todd Lipcon added a comment - +1. Will commit to trunk momentarily (manually ran the one changed test case)
          Hide
          Luke Lu added a comment -

          Patch v2 make the barrier local for better readability.

          Show
          Luke Lu added a comment - Patch v2 make the barrier local for better readability.
          Hide
          Todd Lipcon added a comment -

          aha, I guess I learn something new every day

          Nonetheless since it's only used in that local scope for now, why not minimize it? Makes it easier to understand that it's only triggered from that thread and awaited from the other.

          Show
          Todd Lipcon added a comment - aha, I guess I learn something new every day Nonetheless since it's only used in that local scope for now, why not minimize it? Makes it easier to understand that it's only triggered from that thread and awaited from the other.
          Hide
          Luke Lu added a comment -

          I was planning to use the latch for other tests as well. The latch will not be reused because junit creates a new instance of the test object for each test method: http://martinfowler.com/bliki/JunitNewInstance.html

          Show
          Luke Lu added a comment - I was planning to use the latch for other tests as well. The latch will not be reused because junit creates a new instance of the test object for each test method: http://martinfowler.com/bliki/JunitNewInstance.html
          Hide
          Todd Lipcon added a comment -

          Hi Luke. I think the CountdownLatch should be a local variable in newSleepingConsumerQueue – otherwise, the latch gets reused between test functions.

          Show
          Todd Lipcon added a comment - Hi Luke. I think the CountdownLatch should be a local variable in newSleepingConsumerQueue – otherwise, the latch gets reused between test functions.
          Hide
          Luke Lu added a comment -

          The v1 patch uses proper barrier for thread coordination.

          Show
          Luke Lu added a comment - The v1 patch uses proper barrier for thread coordination.

            People

            • Assignee:
              Luke Lu
              Reporter:
              Luke Lu
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development