[
https://issues.apache.org/jira/browse/HBASE-4321?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13097629#comment-13097629
]
[email protected] commented on HBASE-4321:
------------------------------------------------------
bq. On 2011-09-02 18:31:56, Todd Lipcon wrote:
bq. > src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java,
line 71
bq. > <https://reviews.apache.org/r/1703/diff/1/?file=37633#file37633line71>
bq. >
bq. > typo "is contains"
fixed
bq. On 2011-09-02 18:31:56, Todd Lipcon wrote:
bq. > src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java,
line 87
bq. > <https://reviews.apache.org/r/1703/diff/1/?file=37633#file37633line87>
bq. >
bq. > remove empty javadoc
done
bq. On 2011-09-02 18:31:56, Todd Lipcon wrote:
bq. > src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java,
line 90
bq. > <https://reviews.apache.org/r/1703/diff/1/?file=37633#file37633line90>
bq. >
bq. > rename "obj" to "range" or something
range
bq. On 2011-09-02 18:31:56, Todd Lipcon wrote:
bq. > src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java,
line 95
bq. > <https://reviews.apache.org/r/1703/diff/1/?file=37633#file37633line95>
bq. >
bq. > maybe log a warning? or at least a debug.
bq. >
bq. > Should this be >= 0? I don't know if other stuff would fall apart if
he had a degenerate/empty region
debug.
bq. On 2011-09-02 18:31:56, Todd Lipcon wrote:
bq. > src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java,
lines 123-136
bq. > <https://reviews.apache.org/r/1703/diff/1/?file=37633#file37633line123>
bq. >
bq. > if I understand this correctly, it might be easier written as
something like:
bq. >
bq. > for (byte[] coveredSplit : splits.subSet(r.getStartKey(),
r.endKey()) {
bq. > regions.put(coveredSplit, r);
bq. > }
bq. >
bq. >
nice. done.
bq. On 2011-09-02 18:31:56, Todd Lipcon wrote:
bq. >
src/test/java/org/apache/hadoop/hbase/util/TestRegionSplitCalculator.java, line
104
bq. > <https://reviews.apache.org/r/1703/diff/1/?file=37634#file37634line104>
bq. >
bq. > 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:
bq. >
bq. > assertEquals(
bq. > "A:t[A,B]\n" +
bq. > "B:\t[B,C]\t[B,D]\n" +
bq. > "D:\t\n" +
bq. > "E:\t[E,F]\n",
bq. > dump(foo));
Sure.
bq. On 2011-09-02 18:31:56, Todd Lipcon wrote:
bq. > src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java,
line 73
bq. > <https://reviews.apache.org/r/1703/diff/1/?file=37633#file37633line73>
bq. >
bq. > what's n?
number of ranges added. comment wasn't completely accurate, updated.
bq. On 2011-09-02 18:31:56, Todd Lipcon wrote:
bq. > src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java,
line 56
bq. > <https://reviews.apache.org/r/1703/diff/1/?file=37633#file37633line56>
bq. >
bq. > Instead of this, why not make:
bq. >
bq. > interface KeyRange {
bq. > byte[] getStartKey();
bq. > byte[] getEndKey();
bq. > }
bq. >
bq. > and then a Comparator<KeyRange>? Then HRegionInfo could implement
KeyRange with no wrappers.
I've gone down the path of doing it the way you suggested. There is a new
special case that bleeds into this patch, but it looks reasonably clean. This
has to do with an empty end key being an absolute end key.
The overall solution ideally would be to make the end marker be a byte array of
0xff's. This would make everything consistent. (but breaks compatibility).
Another question: I know I've had problems modifying Writables in the past.
Not sure if changing it signature breaks the loading of pre-existing serialized
data. It doesn't affect what I've written for HBASE-4322 but may for the
HRegionInfos. Would changing a signature of existing serialized objects cause
and backwards compatibility problems?
bq. On 2011-09-02 18:31:56, Todd Lipcon wrote:
bq. > src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java,
lines 62-66
bq. > <https://reviews.apache.org/r/1703/diff/1/?file=37633#file37633line62>
bq. >
bq. > check out ComparisonChain from guava. Something like:
bq. > ComparisonChain.start()
bq. > .compare(getStartKey(), r2.getStartKey())
bq. > .compare(getEndKey(), r2.getEndKey())
bq. > .result()
Looks pretty but seems harder to debugging. Did it anyway.
- jmhsieh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1703/#review1735
-----------------------------------------------------------
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
