[
https://issues.apache.org/jira/browse/HBASE-1923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12873486#action_12873486
]
HBase Review Board commented on HBASE-1923:
-------------------------------------------
Message from: "Todd Lipcon" <[email protected]>
bq. On 2010-05-28 17:19:22, stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java, line 264
bq. > <http://review.hbase.org/r/87/diff/1/?file=610#file610line264>
bq. >
bq. > Is this patch for trunk or branch? If branch, this addition might
break RPC. It might be ok on the end here but we should test.
I will be backporting to branch for our customer, but as a major new feature I
didn't expect it would be committed to Apache branch. If people want it,
though, I'll put the patch up.
In general, adding a new RPC doesn't cause incompatibility - if a client calls
it and it's not there on the server side, the client will just get an exception
that the method doesn't exist. (eg see my backport of HDFS-630 to hadoop 20)
bq. On 2010-05-28 17:19:22, stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java,
line 181
bq. > <http://review.hbase.org/r/87/diff/1/?file=612#file612line181>
bq. >
bq. > Man, we never enable asserts.... throw an exception!
bq. >
Switched to using google Preconditions
bq. On 2010-05-28 17:19:22, stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java,
line 187
bq. > <http://review.hbase.org/r/87/diff/1/?file=612#file612line187>
bq. >
bq. > Just asking, the comparator for sure is going to do the right thing
here?
yea, ImmutableBytesWritable implements Comparable
bq. On 2010-05-28 17:19:22, stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java,
line 213
bq. > <http://review.hbase.org/r/87/diff/1/?file=612#file612line213>
bq. >
bq. > Won't javadoc mangle your nice formatting here? Use ul/li or wrap
w/ <pre>?
good call, fixed to ul/li
bq. On 2010-05-28 17:19:22, stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat.java,
line 234
bq. > <http://review.hbase.org/r/87/diff/1/?file=612#file612line234>
bq. >
bq. > Why two types? Is this legacy? KV has advantage of being able to
carry a Delete
yea, it's for compatibility with TableOutputFormat which can take either. Kind
of a pain, maybe we should just get rid of it and only accept KV?
bq. On 2010-05-28 17:19:22, stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/mapreduce/ImportTsv.java, line 73
bq. > <http://review.hbase.org/r/87/diff/1/?file=613#file613line73>
bq. >
bq. > In KV, there is a parseColumns function that might help here
doesn't seem much more convenient (since I already have String here)
One question, though: if I want to insert into a family that has no qualifiers,
should I be using EMPTY_BYTE_ARRAY when I construct the KV, or passing null?
bq. On 2010-05-28 17:19:22, stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/mapreduce/ImportTsv.java, line 96
bq. > <http://review.hbase.org/r/87/diff/1/?file=613#file613line96>
bq. >
bq. > Is this safe? Is there escaping you should be handling here?
Plan to address this in docs for importtsv - it's not a good TSV parser that
supports quoting, etc.
bq. On 2010-05-28 17:19:22, stack wrote:
bq. >
src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java,
line 229
bq. > <http://review.hbase.org/r/87/diff/1/?file=614#file614line229>
bq. >
bq. > I suppose in rare circumstances, it could split in here and mess you
up. You need a no-split flag per region out in zk or something? We'd need
such a thing for snapshotting methinks. But, thats for a later...
if there's been a split, the new daughter region will detect that the HFile
doesn't "fit" and throw WrongRegionException. This then triggers the retry on
ServerCallable, which fetches the new region info from meta, and realizes it
has to split the hfile before trying again.
bq. On 2010-05-28 17:19:22, stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/mapreduce/PutSortReducer.java,
line 29
bq. > <http://review.hbase.org/r/87/diff/1/?file=615#file615line29>
bq. >
bq. > Class comment. Whats this thing do?
fixed - copied from KeyValueSortReducer
bq. On 2010-05-28 17:19:22, stack wrote:
bq. >
src/main/java/org/apache/hadoop/hbase/mapreduce/hadoopbackport/InputSampler.java,
line 57
bq. > <http://review.hbase.org/r/87/diff/1/?file=617#file617line57>
bq. >
bq. > We need this even though you wrote out partition edges earlier when
you read region boundaries?
You're right that it's unused, but I figured I'd put it in for convenience -
when doing a bulk load into a new table, for example, you may want to use this
in order to figure out where to set region splits.
bq. On 2010-05-28 17:19:22, stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line
1897
bq. > <http://review.hbase.org/r/87/diff/1/?file=619#file619line1897>
bq. >
bq. > This is old cruddy stuff you are asking about. It looks like I
added splitsAndClosesLock to stop either happening during critical periods
(HBASE-588) and the splits lock looks now like a half measure from way back in
2007 -- HBASE-217. Lets make an issue to clean it up.
Which one should I be taking? Do I need both? Which order?
bq. On 2010-05-28 17:19:22, stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/Store.java, line 511
bq. > <http://review.hbase.org/r/87/diff/1/?file=621#file621line511>
bq. >
bq. > There is FSUtils in hbase util
On second thought actually this should be somewhat store-specific, so took out
the TODO
bq. On 2010-05-28 17:19:22, stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/util/Bytes.java, line 1105
bq. > <http://review.hbase.org/r/87/diff/1/?file=622#file622line1105>
bq. >
bq. > You were going to remove this
fixed
bq. On 2010-05-28 17:19:22, stack wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/Store.java, line 535
bq. > <http://review.hbase.org/r/87/diff/1/?file=621#file621line535>
bq. >
bq. > What you going to do here? Get the last key from the map of
storefiles in current region and make your file be one less than it? Your
timestamp is current though so edits in here could overshadow those in another
storefile. Thats ok?
going to make storefiles be a list instead of map... new review coming in a bit
bq. On 2010-05-28 17:19:22, stack wrote:
bq. > src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java, line 646
bq. > <http://review.hbase.org/r/87/diff/1/?file=623#file623line646>
bq. >
bq. > Whats this about?
added comment - idea is to restore back to local-mode job runner when cluster
shuts down
bq. On 2010-05-28 17:19:22, stack wrote:
bq. > src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java, line 784
bq. > <http://review.hbase.org/r/87/diff/1/?file=623#file623line784>
bq. >
bq. > Its not removing it itself? Its registered to cleanup on exit.
was seeing cases where in between different tests in the same test case, it
wouldn't get cleaned up, etc.
bq. On 2010-05-28 17:19:22, stack wrote:
bq. > src/test/java/org/apache/hadoop/hbase/mapreduce/NMapInputFormat.java,
line 40
bq. > <http://review.hbase.org/r/87/diff/1/?file=624#file624line40>
bq. >
bq. > How? If nullwritables, don't I just get nulls?
Changed to:
/**
* Input format that creates as many map tasks as configured in
* mapred.map.tasks, each provided with a single row of
* NullWritables. This can be useful when trying to write mappers
* which don't have any real input (eg when the mapper is simply
* producing random data as output)
*/
- Todd
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/87/#review96
-----------------------------------------------------------
> Bulk incremental load into an existing table
> --------------------------------------------
>
> Key: HBASE-1923
> URL: https://issues.apache.org/jira/browse/HBASE-1923
> Project: HBase
> Issue Type: New Feature
> Components: client, mapred, regionserver, scripts
> Affects Versions: 0.21.0
> Reporter: anty.rao
> Assignee: Todd Lipcon
> Attachments: hbase-1923-prelim.txt
>
>
> hbase-48 is about bulk load of a new table,maybe it's more practicable to
> bulk load aganist a existing table.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
