HBase
  1. HBase
  2. HBASE-317

Replacing thread notification-based queue with java.util.concurrent.BlockingQueue in HMaster

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      In HMaster, there is a msgQueue typed as an ArrayList<PendingOperations>. PendingOperations are added to the queue and notify() is called on msgQueue. I think using a BlockingQueue<PendingOperations> makes the intent of the code more clear and removes potentially problematic synchronization issues. BlockingQueues are thread-safe and allow multiple producers and consumers. I have written a patch for this and will attach it.

      1. HADOOP-1615.patch
        6 kB
        Steve Jenson
      2. patch.txt
        8 kB
        Jim Kellerman
      3. patch.txt
        8 kB
        Jim Kellerman

        Activity

        Hide
        Jim Kellerman added a comment -

        Committed patch

        Show
        Jim Kellerman added a comment - Committed patch
        Hide
        Hadoop QA added a comment -

        -1, build or testing failed

        2 attempts failed to build and test the latest attachment http://issues.apache.org/jira/secure/attachment/12361939/patch.txt against trunk revision r556754.

        Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/425/testReport/
        Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/425/console

        Please note that this message is automatically generated and may represent a problem with the automation system and not the patch.

        Show
        Hadoop QA added a comment - -1, build or testing failed 2 attempts failed to build and test the latest attachment http://issues.apache.org/jira/secure/attachment/12361939/patch.txt against trunk revision r556754. Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/425/testReport/ Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/425/console Please note that this message is automatically generated and may represent a problem with the automation system and not the patch.
        Hide
        Jim Kellerman added a comment -

        Updated to correspond to changes in trunk

        Show
        Jim Kellerman added a comment - Updated to correspond to changes in trunk
        Hide
        Steve Jenson added a comment -

        Thanks!

        Also, before I sent this patch in, I tested it several times with:

        hbase org.apache.hadoop.hbase.PerformanceEvaluation sequentialWrite 1

        and measured no performance degredation from a baseline copy I had checked out.

        Show
        Steve Jenson added a comment - Thanks! Also, before I sent this patch in, I tested it several times with: hbase org.apache.hadoop.hbase.PerformanceEvaluation sequentialWrite 1 and measured no performance degredation from a baseline copy I had checked out.
        Hide
        Hadoop QA added a comment -

        -1, build or testing failed

        2 attempts failed to build and test the latest attachment http://issues.apache.org/jira/secure/attachment/12361913/patch.txt against trunk revision r556746.

        Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/420/testReport/
        Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/420/console

        Please note that this message is automatically generated and may represent a problem with the automation system and not the patch.

        Show
        Hadoop QA added a comment - -1, build or testing failed 2 attempts failed to build and test the latest attachment http://issues.apache.org/jira/secure/attachment/12361913/patch.txt against trunk revision r556746. Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/420/testReport/ Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/420/console Please note that this message is automatically generated and may represent a problem with the automation system and not the patch.
        Hide
        Jim Kellerman added a comment -

        Steve,

        Thanks for the offer. I already made the change and submitted the revised patch.

        Show
        Jim Kellerman added a comment - Steve, Thanks for the offer. I already made the change and submitted the revised patch.
        Hide
        Jim Kellerman added a comment -

        Works in my environment. Get Hudson to agree

        Show
        Jim Kellerman added a comment - Works in my environment. Get Hudson to agree
        Hide
        Steve Jenson added a comment -

        I'm happy to make the change in HRegionServer.

        Do you want me to change the timeout value to be configurable?

        Show
        Steve Jenson added a comment - I'm happy to make the change in HRegionServer. Do you want me to change the timeout value to be configurable?
        Hide
        Jim Kellerman added a comment -

        Make similar change in HRegionServer.

        Change timeout value.

        Show
        Jim Kellerman added a comment - Make similar change in HRegionServer. Change timeout value.
        Hide
        Jim Kellerman added a comment -

        Additional changes necessary

        Show
        Jim Kellerman added a comment - Additional changes necessary
        Hide
        Jim Kellerman added a comment -

        This also applies to HRegionServer

        Show
        Jim Kellerman added a comment - This also applies to HRegionServer
        Hide
        Steve Jenson added a comment -

        There was no obvious place to add a patch in the 'Add Patch' workflow so I am uploading it here. Thanks.

        Show
        Steve Jenson added a comment - There was no obvious place to add a patch in the 'Add Patch' workflow so I am uploading it here. Thanks.

          People

          • Assignee:
            Unassigned
            Reporter:
            Steve Jenson
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development