HBase
  1. HBase
  2. HBASE-10722

[0.94] HRegion.computeHDFSBlocksDistribution does not account for links and reference files.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.94.18
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    1. 10722.txt
      3 kB
      Lars Hofhansl

      Issue Links

        Activity

        Hide
        Hudson added a comment -

        FAILURE: Integrated in HBase-0.94 #1321 (See https://builds.apache.org/job/HBase-0.94/1321/)
        HBASE-10722 [0.94] HRegion.computeHDFSBlocksDistribution does not account for links and reference files. (larsh: rev 1577404)

        • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        Show
        Hudson added a comment - FAILURE: Integrated in HBase-0.94 #1321 (See https://builds.apache.org/job/HBase-0.94/1321/ ) HBASE-10722 [0.94] HRegion.computeHDFSBlocksDistribution does not account for links and reference files. (larsh: rev 1577404) /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        Hide
        Hudson added a comment -

        FAILURE: Integrated in HBase-0.94-security #440 (See https://builds.apache.org/job/HBase-0.94-security/440/)
        HBASE-10722 [0.94] HRegion.computeHDFSBlocksDistribution does not account for links and reference files. (larsh: rev 1577404)

        • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        Show
        Hudson added a comment - FAILURE: Integrated in HBase-0.94-security #440 (See https://builds.apache.org/job/HBase-0.94-security/440/ ) HBASE-10722 [0.94] HRegion.computeHDFSBlocksDistribution does not account for links and reference files. (larsh: rev 1577404) /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        Hide
        Hudson added a comment -

        FAILURE: Integrated in HBase-0.94-on-Hadoop-2 #50 (See https://builds.apache.org/job/HBase-0.94-on-Hadoop-2/50/)
        HBASE-10722 [0.94] HRegion.computeHDFSBlocksDistribution does not account for links and reference files. (larsh: rev 1577404)

        • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        Show
        Hudson added a comment - FAILURE: Integrated in HBase-0.94-on-Hadoop-2 #50 (See https://builds.apache.org/job/HBase-0.94-on-Hadoop-2/50/ ) HBASE-10722 [0.94] HRegion.computeHDFSBlocksDistribution does not account for links and reference files. (larsh: rev 1577404) /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        Hide
        Hudson added a comment -

        FAILURE: Integrated in HBase-0.94-JDK7 #84 (See https://builds.apache.org/job/HBase-0.94-JDK7/84/)
        HBASE-10722 [0.94] HRegion.computeHDFSBlocksDistribution does not account for links and reference files. (larsh: rev 1577404)

        • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        Show
        Hudson added a comment - FAILURE: Integrated in HBase-0.94-JDK7 #84 (See https://builds.apache.org/job/HBase-0.94-JDK7/84/ ) HBASE-10722 [0.94] HRegion.computeHDFSBlocksDistribution does not account for links and reference files. (larsh: rev 1577404) /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        Hide
        Lars Hofhansl added a comment -

        Committed to 0.94.

        Show
        Lars Hofhansl added a comment - Committed to 0.94.
        Hide
        Lars Hofhansl added a comment -

        OK. Lemme commit this here. (needed for HBASE-10642)

        Show
        Lars Hofhansl added a comment - OK. Lemme commit this here. (needed for HBASE-10642 )
        Hide
        Enis Soztutar added a comment -

        +1 on fixing it in HRegion. We do not have to clean that up.

        The other place is CompactionInputFormat, and that's also not right (also wrong in trunk).

        It seems like HRegionFS is the right place to make this consumable.

        Show
        Enis Soztutar added a comment - +1 on fixing it in HRegion. We do not have to clean that up. The other place is CompactionInputFormat, and that's also not right (also wrong in trunk). It seems like HRegionFS is the right place to make this consumable.
        Hide
        Lars Hofhansl added a comment -

        Thanks Nick.

        Hmm... StoreFile.computeRefFileHDFSBlockDistribution is also calling that, and doing the right thing (in fact that's where I looked as how it's done correctly).
        The other place is CompactionInputFormat, and that's also not right (also wrong in trunk).
        I think FSUtil should stay at the FS level, and not know much about HBase specifics. In trunk we have HRegionFileSystem, but not in 0.94. Then again in trunk I see stuff like getRegionDegreeLocalityMappingFromFS in FSUtil. What a mess.

        I think for 0.94 it's best to fix it in HRegion. I would just remove it, but coprocessor might call it (and currently get wrong results).

        Show
        Lars Hofhansl added a comment - Thanks Nick. Hmm... StoreFile.computeRefFileHDFSBlockDistribution is also calling that, and doing the right thing (in fact that's where I looked as how it's done correctly). The other place is CompactionInputFormat, and that's also not right (also wrong in trunk). I think FSUtil should stay at the FS level, and not know much about HBase specifics. In trunk we have HRegionFileSystem, but not in 0.94. Then again in trunk I see stuff like getRegionDegreeLocalityMappingFromFS in FSUtil. What a mess. I think for 0.94 it's best to fix it in HRegion. I would just remove it, but coprocessor might call it (and currently get wrong results).
        Hide
        Nick Dimiduk added a comment -

        FSUtils#computeHDFSBlocksDistribution looks like the more commonly used implementation. The fix should go there and this method removed from HRegion.

        Patch looks good otherwise

        Show
        Nick Dimiduk added a comment - FSUtils#computeHDFSBlocksDistribution looks like the more commonly used implementation. The fix should go there and this method removed from HRegion. Patch looks good otherwise
        Hide
        Lars Hofhansl added a comment -

        Please have a look.

        Then again, maybe this is pointless, HRegion.computeHDFSBlocksDistribution is only called from two places:

        1. TestHRegion
        2. DefaultLoadBalancer.getTopBlockLocations (which is private is not called from anywhere ?!)

        Seems the entire method could just as well be removed.

        Show
        Lars Hofhansl added a comment - Please have a look. Then again, maybe this is pointless, HRegion.computeHDFSBlocksDistribution is only called from two places: TestHRegion DefaultLoadBalancer.getTopBlockLocations (which is private is not called from anywhere ?!) Seems the entire method could just as well be removed.

          People

          • Assignee:
            Lars Hofhansl
            Reporter:
            Lars Hofhansl
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development