HBase
  1. HBase
  2. HBASE-5482

In 0.90, balancer algo leading to same region balanced twice and picking same region with Src and Destination as same RS.

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.90.5
    • Fix Version/s: 0.90.7
    • Component/s: master
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      There are possibility of 2 problems
      -> When we populate regionsToMove while iterating the serverinfo in descending manner there is a chance that the same region can be added twice.
      Because in the first loop we do a randomization of the regions.
      Where as when we get we have neededRegions!= 0 we just get the region in the index and add it again . This may lead to have same region in the regionsToMove list.
      -> Another problem is
      when the problem in the first point happens then there is a chance that
      the regionToMove can have the same src and destination and the same region can be picked every 5 mins.

      for(Map.Entry<HServerInfo, List<HRegionInfo>> server :
              serversByLoad.descendingMap().entrySet()) {
              BalanceInfo balanceInfo = serverBalanceInfo.get(server.getKey());
              int idx =
                balanceInfo == null ? 0 : balanceInfo.getNextRegionForUnload();
              if (idx >= server.getValue().size()) break;
              HRegionInfo region = server.getValue().get(idx);
              if (region.isMetaRegion()) continue; // Don't move meta regions.
              regionsToMove.add(new RegionPlan(region, server.getKey(), null));
              if(--neededRegions == 0) {
                // No more regions needed, done shedding
                break;
              }
            }
      

      If i have meta and root in the top two loaded region server(totally 3 RS), we just skip the regions in those region server and populate the region from the least loaded RS.
      Then in the next loop we iterate from the least loaded server and populate the destination as also the same server.
      This is leading to a condition where every 5 min balancing happens and also the server is same for src and dest.

      1. HBASE-5482_2.patch
        12 kB
        ramkrishna.s.vasudevan
      2. HBASE-5482_1.patch
        12 kB
        ramkrishna.s.vasudevan
      3. 5482-v2.txt
        12 kB
        Ted Yu

        Activity

        Hide
        ramkrishna.s.vasudevan added a comment -

        Committed to 0.90. Thanks for the review Ted.

        Show
        ramkrishna.s.vasudevan added a comment - Committed to 0.90. Thanks for the review Ted.
        Hide
        Ted Yu added a comment -

        The following tests passed based on patch v2:

          929  mt -Dtest=TestLoadBalancer
          931  mt -Dtest=TestRegionRebalancing
        

        +1 on patch v2.

        Show
        Ted Yu added a comment - The following tests passed based on patch v2: 929 mt -Dtest=TestLoadBalancer 931 mt -Dtest=TestRegionRebalancing +1 on patch v2.
        Hide
        ramkrishna.s.vasudevan added a comment -

        Patch addressing Ted's comments.
        @Ted
        If the patch is fine, we can commit it for 0.90.7?

        Show
        ramkrishna.s.vasudevan added a comment - Patch addressing Ted's comments. @Ted If the patch is fine, we can commit it for 0.90.7?
        Hide
        Ted Yu added a comment -

        Patch looks good.

        Please add curly braces around break statement:

        +          if (idx >= server.getValue().size())
        +            break;
        

        In:

        +  public void testBalanceClusterWithFirstRegionsRootMeta() throws Exception {
        

        the following should end with 'META are':

        +      // do not randomize in order to create a scenario where ROOT and META is 
        
        +  public void testPlanContainsDuplicateRegions() throws Exception {
        

        Would testNoDuplicateRegionsInPlan() be a good name ?

        Show
        Ted Yu added a comment - Patch looks good. Please add curly braces around break statement: + if (idx >= server.getValue().size()) + break ; In: + public void testBalanceClusterWithFirstRegionsRootMeta() throws Exception { the following should end with 'META are': + // do not randomize in order to create a scenario where ROOT and META is + public void testPlanContainsDuplicateRegions() throws Exception { Would testNoDuplicateRegionsInPlan() be a good name ?
        Hide
        Ted Yu added a comment -

        Patch v2 corrected grammar in comments

        Show
        Ted Yu added a comment - Patch v2 corrected grammar in comments
        Hide
        ramkrishna.s.vasudevan added a comment -

        Checking if the patch similar to this is needed in trunk. Once confirmed will prepare a patch for 0.92.

        Show
        ramkrishna.s.vasudevan added a comment - Checking if the patch similar to this is needed in trunk. Once confirmed will prepare a patch for 0.92.
        Hide
        ramkrishna.s.vasudevan added a comment -

        Patch for 0.90.

        Show
        ramkrishna.s.vasudevan added a comment - Patch for 0.90.

          People

          • Assignee:
            ramkrishna.s.vasudevan
            Reporter:
            ramkrishna.s.vasudevan
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development