sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anna Szonyi <szo...@cloudera.com>
Subject Re: Review Request 57499: Sqoop incremental import - NULL column updates are not pulled into HBase table
Date Wed, 07 Jun 2017 16:50:37 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57499/#review173716
-----------------------------------------------------------



Hi,

Thank you for this patch, this seems like a very useful change! There are a few suggestions/questions
I've added, please look through them and fix/comment as needed :). Also thank you for creating
a test case for this, that was a lot of help with the review and testing!

Thanks,
Anna


src/java/org/apache/sqoop/hbase/ToStringPutTransformer.java
Line 191 (original), 196 (patched)
<https://reviews.apache.org/r/57499/#comment250697>

    it seems Put class' add method was deprecated in hbase 1.0.0 and we're currently using
hbase 1.2.4, please use put.addColumn instead.



src/java/org/apache/sqoop/hbase/ToStringPutTransformer.java
Lines 205 (patched)
<https://reviews.apache.org/r/57499/#comment250696>

    it seems deleteColumn was deprecated in hbase 1.0.0 and we're currently using hbase 1.2.4,
please use delete.addColumn instead.



src/java/org/apache/sqoop/hbase/ToStringPutTransformer.java
Lines 211 (patched)
<https://reviews.apache.org/r/57499/#comment246762>

    It might be cleaner if you initialize the list of mutations sooner and add the correct
put or delete to the list based on the val == null instead of carrying the booleans isPut
and isDelete (or create a mutation object and carry that around, in that case you can create
an immutable list instead of a regular ArrayList).



src/test/com/cloudera/sqoop/hbase/HBaseImportTest.java
Lines 85 (patched)
<https://reviews.apache.org/r/57499/#comment250725>

    Currently we're only testing the null update for lastmodified. Based on my understanding
this should not affect append mode, could you please add a test case to verify that this change
does not affect append mode, if you agree with my assumption.



src/test/com/cloudera/sqoop/hbase/HBaseTestCase.java
Line 118 (original), 125-156 (patched)
<https://reviews.apache.org/r/57499/#comment250724>

    Quite a bit of this function is already in the original getArgv function in the HBaseTestCase
class, please reuse/extract-refactor that part of the method.


- Anna Szonyi


On April 9, 2017, 5:49 p.m., Jilani Shaik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57499/
> -----------------------------------------------------------
> 
> (Updated April 9, 2017, 5:49 p.m.)
> 
> 
> Review request for Sqoop, Boglarka Egyed, Attila Szabo, and Anna Szonyi.
> 
> 
> Bugs: SQOOP-3149
>     https://issues.apache.org/jira/browse/SQOOP-3149
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> -------
> 
> HBase delete is added as part of incremental data import, Modified the return type of
method which is responsible for holding list of Put objects to List of Mutation objects and
at the time of execution of insert or delete using the type of object in Mutation, executed
the actaul operation either insert or delete.
> 
> Jira ticket for the same: https://issues.apache.org/jira/browse/SQOOP-3149
> 
> Similar ticket to above:  SQOOP-3125
> 
> 
> Diffs
> -----
> 
>   src/java/org/apache/sqoop/hbase/HBasePutProcessor.java fdbe1276 
>   src/java/org/apache/sqoop/hbase/PutTransformer.java 533467e5 
>   src/java/org/apache/sqoop/hbase/ToStringPutTransformer.java 363e1456 
>   src/java/org/apache/sqoop/mapreduce/HBaseBulkImportMapper.java 58ccee7b 
>   src/test/com/cloudera/sqoop/hbase/HBaseImportTest.java fa14a013 
>   src/test/com/cloudera/sqoop/hbase/HBaseTestCase.java a054eb66 
>   src/test/com/cloudera/sqoop/testutil/BaseSqoopTestCase.java 6310a398 
> 
> 
> Diff: https://reviews.apache.org/r/57499/diff/4/
> 
> 
> Testing
> -------
> 
> Executed with jar prepared with these changes into hadoop cluster and tested bot import
and then incremental import.
> 
> 
> File Attachments
> ----------------
> 
> HBase delete support for incremental import
>   https://reviews.apache.org/media/uploaded/files/2017/03/11/5b1895fd-4c6b-42fa-8a92-4e093153e370__hbase_delete_support_in_incremental_import
> updated with unit test and based on below suggestions
>   https://reviews.apache.org/media/uploaded/files/2017/03/22/708f63ba-2d8a-4a47-ab67-c1d2776354fd__hbase_delete_support_in_incremental_unit_test_also
> 
> 
> Thanks,
> 
> Jilani Shaik
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message