[ https://issues.apache.org/jira/browse/HBASE-5719?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13250087#comment-13250087 ]
jirapos...@reviews.apache.org commented on HBASE-5719: ------------------------------------------------------ bq. On 2012-04-09 18:20:11, jmhsieh wrote: bq. > src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java, line 1869 bq. > <https://reviews.apache.org/r/4649/diff/2/?file=100672#file100672line1869> bq. > bq. > Maybe this one is at info level? Will change to info level. bq. On 2012-04-09 18:20:11, jmhsieh wrote: bq. > src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java, line 2828 bq. > <https://reviews.apache.org/r/4649/diff/2/?file=100672#file100672line2828> bq. > bq. > nit: typo (old was correct) Changed it back. It must be because the mouse moved when I was typing something. I didn't remember I changed it. :) bq. On 2012-04-09 18:20:11, jmhsieh wrote: bq. > src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java, line 2895 bq. > <https://reviews.apache.org/r/4649/diff/2/?file=100672#file100672line2895> bq. > bq. > Is this <n> per group or globally? please make it clear in usage It is per group. Will fix it. bq. On 2012-04-09 18:20:11, jmhsieh wrote: bq. > src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java, line 2981 bq. > <https://reviews.apache.org/r/4649/diff/2/?file=100672#file100672line2981> bq. > bq. > nit: should be like previous -maxMerge's comments. Will fix it. bq. On 2012-04-09 18:20:11, jmhsieh wrote: bq. > src/test/java/org/apache/hadoop/hbase/util/TestRegionSplitCalculator.java, line 360 bq. > <https://reviews.apache.org/r/4649/diff/2/?file=100674#file100674line360> bq. > bq. > nit: it would easer to read if this was called ai, the next ae, and the one after that ac. bq. > bq. > Also would help if you put the expected overlap count in comments! Sure. bq. On 2012-04-09 18:20:11, jmhsieh wrote: bq. > src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java, line 212 bq. > <https://reviews.apache.org/r/4649/diff/2/?file=100673#file100673line212> bq. > bq. > what is the intuition here? Added comment bq. On 2012-04-09 18:20:11, jmhsieh wrote: bq. > src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java, line 189 bq. > <https://reviews.apache.org/r/4649/diff/2/?file=100673#file100673line189> bq. > bq. > add comment? bq. > bq. > We always overlap with ourselves -- which is why > 1 is the condition below.. Will do. bq. On 2012-04-09 18:20:11, jmhsieh wrote: bq. > src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java, line 178 bq. > <https://reviews.apache.org/r/4649/diff/2/?file=100673#file100673line178> bq. > bq. > add comment: calculates the # of overlaps for each region and populates rangeOverlapMap Sure bq. On 2012-04-09 18:20:11, jmhsieh wrote: bq. > src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java, line 177 bq. > <https://reviews.apache.org/r/4649/diff/2/?file=100673#file100673line177> bq. > bq. > Add comment about structure contents. something like: Integer key is overlap count and List is regions that have that many overlaps. bq. > maybe rename to overlapRangeMap? Sure. bq. On 2012-04-09 18:20:11, jmhsieh wrote: bq. > src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java, line 171 bq. > <https://reviews.apache.org/r/4649/diff/2/?file=100673#file100673line171> bq. > bq. > To find? Would returning a sort make sense? if there are no overlaps will this return the specified number (or less?) bq. > bq. > Could this return several regions that now leave holes? bq. > bq. > nit: It might make testing clearer to return the overlapCount -> regions map and check that. Added comment. - Jimmy ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4649/#review6764 ----------------------------------------------------------- On 2012-04-06 18:27:34, Jimmy Xiang wrote: bq. bq. ----------------------------------------------------------- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/4649/ bq. ----------------------------------------------------------- bq. bq. (Updated 2012-04-06 18:27:34) bq. bq. bq. Review request for hbase and jmhsieh. bq. bq. bq. Summary bq. ------- bq. bq. Make it configurable to sideline some regions in big overlapped groups which hbck doesn't handle currently. bq. bq. The regions chose to sideline are those which overlap with most other regions. bq. bq. bq. This addresses bug HBASE-5719. bq. https://issues.apache.org/jira/browse/HBASE-5719 bq. bq. bq. Diffs bq. ----- bq. bq. src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 54f9b21 bq. src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java 17678dd bq. src/test/java/org/apache/hadoop/hbase/util/TestRegionSplitCalculator.java ac3b225 bq. bq. Diff: https://reviews.apache.org/r/4649/diff bq. bq. bq. Testing bq. ------- bq. bq. mvn -PlocalTests -Dtest=TestHBaseFsck* clean test bq. bq. Also tested in real system to fix inconsistencies. bq. bq. bq. Thanks, bq. bq. Jimmy bq. bq. > Enhance hbck to sideline overlapped mega regions > ------------------------------------------------ > > Key: HBASE-5719 > URL: https://issues.apache.org/jira/browse/HBASE-5719 > Project: HBase > Issue Type: New Feature > Components: hbck > Affects Versions: 0.94.0, 0.96.0 > Reporter: Jimmy Xiang > Assignee: Jimmy Xiang > Fix For: 0.96.0 > > Attachments: hbase-5719.patch > > > If there are too many regions in one overlapped group (by default, more than > 10), hbck currently doesn't merge them since it takes time. > In this case, we can sideline some regions in the group and break the > overlapping to fix the inconsistency. Later on, sidelined regions can be > bulk loaded manually. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira