HBase
  1. HBase
  2. HBASE-3401

Region IPC operations should be high priority

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.90.1
    • Fix Version/s: 0.90.0
    • Component/s: regionserver
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      I manufactured an imbalanced cluster so one region server had 300 regions and the others had very few. I then ran balancer while hitting the high-load region server with YCSB. I observed that the rate of load shedding was VERY slow since the closeRegion IPCs were getting stuck at the back of the IPC queue.

      All of these important master->RS RPC calls should be set to high priority.

      1. hbase-3401.txt
        10 kB
        Todd Lipcon
      2. hbase-3401.txt
        7 kB
        Todd Lipcon

        Activity

        Hide
        Todd Lipcon added a comment -

        Here's a patch which adds a new QosPriority annotation which can be used to prioritize things more easily right in the interface.

        We can eventually extend this to be more generalized, but this should work for now and make it more obvious when adding new RPCs how to prioritize them.

        Show
        Todd Lipcon added a comment - Here's a patch which adds a new QosPriority annotation which can be used to prioritize things more easily right in the interface. We can eventually extend this to be more generalized, but this should work for now and make it more obvious when adding new RPCs how to prioritize them.
        Hide
        Todd Lipcon added a comment -

        Tested on cluster for basic operation, though have not tested that scenario again yet.

        Show
        Todd Lipcon added a comment - Tested on cluster for basic operation, though have not tested that scenario again yet.
        Hide
        Todd Lipcon added a comment -

        oops, this patch doesn't work... will upload a new one soon.

        Show
        Todd Lipcon added a comment - oops, this patch doesn't work... will upload a new one soon.
        Hide
        stack added a comment -

        Thats pretty fancy Todd using annotations. You going to upload a new version? Why don't it work?

        Show
        stack added a comment - Thats pretty fancy Todd using annotations. You going to upload a new version? Why don't it work?
        Hide
        Todd Lipcon added a comment -

        Old patch didn't work because annotations aren't inherited from interfaces into the methods in the actual class, which is where I was trying to read them from.

        New patch moves the whole shebang into HRegionServer and appears to actually work

        Show
        Todd Lipcon added a comment - Old patch didn't work because annotations aren't inherited from interfaces into the methods in the actual class, which is where I was trying to read them from. New patch moves the whole shebang into HRegionServer and appears to actually work
        Hide
        stack added a comment -

        I don't think I've seen this construct before:

        +  private @interface QosPriority {
        +    int priority() default 0;
        +  }
        

        No curlies around the default 0? This how annotations do 'interfaces'?

        Will this all work if someone subclasses and overrides HRS methods?

        Otherwise, +1 on commit. Looks good to me.

        Show
        stack added a comment - I don't think I've seen this construct before: + private @ interface QosPriority { + int priority() default 0; + } No curlies around the default 0? This how annotations do 'interfaces'? Will this all work if someone subclasses and overrides HRS methods? Otherwise, +1 on commit. Looks good to me.
        Hide
        Todd Lipcon added a comment -

        Stack: yea, the syntax for defining an annotation is wacky. I had to look it up mysefl.

        Will this all work if someone subclasses and overrides HRS methods?

        Yes, it should - getAnnotation() will return inherited annotations as well. Though perhaps it should be made a protected @interface instead of private for subclassers?

        Though, I thought we were discouraging subclassing these days in favor of CPs?

        Show
        Todd Lipcon added a comment - Stack: yea, the syntax for defining an annotation is wacky. I had to look it up mysefl. Will this all work if someone subclasses and overrides HRS methods? Yes, it should - getAnnotation() will return inherited annotations as well. Though perhaps it should be made a protected @interface instead of private for subclassers? Though, I thought we were discouraging subclassing these days in favor of CPs?
        Hide
        stack added a comment -

        Though, I thought we were discouraging subclassing these days in favor of CPs?

        Yes. Doesn't sound like it an issue anyways.

        +1 on commit.

        Show
        stack added a comment - Though, I thought we were discouraging subclassing these days in favor of CPs? Yes. Doesn't sound like it an issue anyways. +1 on commit.
        Hide
        stack added a comment -

        Adding fix in 0.90.0

        Show
        stack added a comment - Adding fix in 0.90.0
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK #1708 (See https://hudson.apache.org/hudson/job/HBase-TRUNK/1708/)

        Show
        Hudson added a comment - Integrated in HBase-TRUNK #1708 (See https://hudson.apache.org/hudson/job/HBase-TRUNK/1708/ )

          People

          • Assignee:
            Todd Lipcon
            Reporter:
            Todd Lipcon
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development