[
https://issues.apache.org/jira/browse/HBASE-4321?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13096189#comment-13096189
]
[email protected] commented on HBASE-4321:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1703/#review1735
-----------------------------------------------------------
src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java
<https://reviews.apache.org/r/1703/#comment3957>
Instead of this, why not make:
interface KeyRange {
byte[] getStartKey();
byte[] getEndKey();
}
and then a Comparator<KeyRange>? Then HRegionInfo could implement KeyRange
with no wrappers.
src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java
<https://reviews.apache.org/r/1703/#comment3949>
check out ComparisonChain from guava. Something like:
ComparisonChain.start()
.compare(getStartKey(), r2.getStartKey())
.compare(getEndKey(), r2.getEndKey())
.result()
src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java
<https://reviews.apache.org/r/1703/#comment3950>
typo "is contains"
src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java
<https://reviews.apache.org/r/1703/#comment3951>
what's n?
src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java
<https://reviews.apache.org/r/1703/#comment3952>
remove empty javadoc
src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java
<https://reviews.apache.org/r/1703/#comment3953>
rename "obj" to "range" or something
src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java
<https://reviews.apache.org/r/1703/#comment3954>
maybe log a warning? or at least a debug.
Should this be >= 0? I don't know if other stuff would fall apart if he had
a degenerate/empty region
src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java
<https://reviews.apache.org/r/1703/#comment3955>
if I understand this correctly, it might be easier written as something
like:
for (byte[] coveredSplit : splits.subSet(r.getStartKey(), r.endKey()) {
regions.put(coveredSplit, r);
}
src/test/java/org/apache/hadoop/hbase/util/TestRegionSplitCalculator.java
<https://reviews.apache.org/r/1703/#comment3956>
I think it would be nice if you actually had this return sb.toString(), and
then for each of the cases, actually asserted equality against something you've
hardcoded. eg:
assertEquals(
"A:t[A,B]\n" +
"B:\t[B,C]\t[B,D]\n" +
"D:\t\n" +
"E:\t[E,F]\n",
dump(foo));
- Todd
On 2011-09-02 15:28:35, jmhsieh wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/1703/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-09-02 15:28:35)
bq.
bq.
bq. Review request for hbase, Todd Lipcon and Michael Stack.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. This is the core region split calculator. It provides a sorted set of
region split points, and a "coverage" multimap. This is enough information to
properly determine dupe start keys, all the different kinds of overlap, as well
as holes.
bq.
bq.
bq. This addresses bug hbase-4321.
bq. https://issues.apache.org/jira/browse/hbase-4321
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java
PRE-CREATION
bq.
src/test/java/org/apache/hadoop/hbase/util/TestRegionSplitCalculator.java
PRE-CREATION
bq.
bq. Diff: https://reviews.apache.org/r/1703/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. Unit tests just checks algorithm, no dependencies on HBase yet.
bq.
bq.
bq. Thanks,
bq.
bq. jmhsieh
bq.
bq.
> Add more comprehensive region split calculator
> ----------------------------------------------
>
> Key: HBASE-4321
> URL: https://issues.apache.org/jira/browse/HBASE-4321
> Project: HBase
> Issue Type: Improvement
> Affects Versions: 0.90.4
> Reporter: Jonathan Hsieh
> Assignee: Jonathan Hsieh
> Attachments:
> 0001-HBASE-4321-Add-more-comprehensive-region-split-calcu.patch
>
>
> Hbck currently scans through meta one entry at a time, only keeping a
> reference to the previous meta entry. This is insufficient for capturing all
> the possible problems in meta and needs something more to properly identify
> holes, overlaps, duplicate start keys, and otherwise invalid meta entries.
> Ideally, this calculator could also be used online interrogating an existing
> meta (HBASE-4058), and also used to generate a completely new meta offline
> just from regioninfo and in hdfs (HBASE-3505).
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
