-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/87/#review108
-----------------------------------------------------------

Ship it!


This is a killer feature.  Ship it.  There is one nitpick in the below but can 
be done on commit (if its legit)


src/main/java/org/apache/hadoop/hbase/io/ImmutableBytesWritable.java
<http://review.hbase.org/r/87/#comment658>

    duh



src/main/java/org/apache/hadoop/hbase/mapreduce/ImportTsv.java
<http://review.hbase.org/r/87/#comment659>

    Where is the javadoc that the tsv parse doesn't do escaping?



src/main/java/org/apache/hadoop/hbase/mapreduce/ImportTsv.java
<http://review.hbase.org/r/87/#comment660>

    Good



src/docs/src/documentation/content/xdocs/bulk-loads.xml
<http://review.hbase.org/r/87/#comment655>

    This is excellent.  We do have this though already: 
http://hbase.apache.org/docs/r0.20.4/api/org/apache/hadoop/hbase/mapreduce/package-summary.html#bulk
    
    We should scrap the above if we're going to go w/ this or update the former 
to include this.



src/docs/src/documentation/content/xdocs/bulk-loads.xml
<http://review.hbase.org/r/87/#comment656>

    method_s_



src/docs/src/documentation/content/xdocs/bulk-loads.xml
<http://review.hbase.org/r/87/#comment657>

    This is a killer feature


- stack


On 2010-06-01 00:33:30, Todd Lipcon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.hbase.org/r/87/
> -----------------------------------------------------------
> 
> (Updated 2010-06-01 00:33:30)
> 
> 
> Review request for hbase, stack and Jonathan Gray.
> 
> 
> Summary
> -------
> 
> Here's a first patch that implements bulk import into existing tables. This 
> applies on top of HBASE-2586 and HBASE-2588 - I've pushed the series of the 
> three to my github: http://github.com/toddlipcon/hbase/tree/hfof-review
> 
> I have some TODOs left that I want to take care of before this gets 
> committed, but since it's a pretty large patch, I figured I'd get it out for 
> review ASAP.
> 
> The stuff in the hadoopbackport package is essentially copypaste from Hadoop 
> trunk, so you can ignore that in the review.
> 
> 
> This addresses bug HBASE-1923.
>     http://issues.apache.org/jira/browse/HBASE-1923
> 
> 
> Diffs
> -----
> 
>   pom.xml 0a009cf 
>   src/docs/src/documentation/content/xdocs/bulk-loads.xml PRE-CREATION 
>   src/docs/src/documentation/content/xdocs/site.xml 0d644f5 
>   src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 29b0cd6 
>   src/main/java/org/apache/hadoop/hbase/io/ImmutableBytesWritable.java 
> 0a9ec4b 
>   src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java b912a85 
>   src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 4cbe52a 
>   src/main/java/org/apache/hadoop/hbase/mapreduce/Driver.java 3d40695 
>   src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java 
> 9c8e53e 
>   src/main/java/org/apache/hadoop/hbase/mapreduce/ImportTsv.java PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java 
> PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/mapreduce/PutSortReducer.java 
> PRE-CREATION 
>   
> src/main/java/org/apache/hadoop/hbase/mapreduce/SimpleTotalOrderPartitioner.java
>  af3d588 
>   src/main/java/org/apache/hadoop/hbase/mapreduce/TableMapReduceUtil.java 
> 07d7911 
>   
> src/main/java/org/apache/hadoop/hbase/mapreduce/hadoopbackport/InputSampler.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/hadoop/hbase/mapreduce/hadoopbackport/TotalOrderPartitioner.java
>  PRE-CREATION 
>   src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 287cd48 
>   src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 
> 78f3223 
>   src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 7de766d 
>   src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 80bf09a 
>   src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java 
> c8941f1 
>   src/main/java/org/apache/hadoop/hbase/util/Bytes.java a53dafe 
>   src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java ed8709f 
>   src/test/java/org/apache/hadoop/hbase/mapreduce/NMapInputFormat.java 
> PRE-CREATION 
>   src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java 
> d04ced2 
>   src/test/java/org/apache/hadoop/hbase/mapreduce/TestImportTsv.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFiles.java
>  PRE-CREATION 
>   src/test/java/org/apache/hadoop/hbase/regionserver/TestCompaction.java 
> f1566d3 
>   src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionInfo.java 
> fcb22fb 
>   src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java 4595e6e 
>   src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java 
> 2e4c7df 
>   src/test/java/org/apache/hadoop/hbase/util/TestBytes.java c7361cb 
> 
> Diff: http://review.hbase.org/r/87/diff
> 
> 
> Testing
> -------
> 
> Primary unit/functional testing, a bit of pseudo-distributed testing. Plan on 
> doing full system tests before commit as well.
> 
> 
> Thanks,
> 
> Todd
> 
>

Reply via email to