hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Thejas Nair <the...@hortonworks.com>
Subject Re: Review Request 54826: HIVE-15448: ChangeManager for replication
Date Tue, 20 Dec 2016 20:56:34 GMT

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




common/src/java/org/apache/hadoop/hive/conf/HiveConf.java (line 440)
<https://reviews.apache.org/r/54826/#comment230534>

    lets keep this disabled for now as this work is in early stages



common/src/java/org/apache/hadoop/hive/conf/HiveConf.java (line 442)
<https://reviews.apache.org/r/54826/#comment230535>

    any HCFS should work for this (Hadoop compatible file system) (at least eventually). I
think we can remove the HDFS reference.



common/src/java/org/apache/hadoop/hive/conf/HiveConf.java (line 443)
<https://reviews.apache.org/r/54826/#comment230536>

    should we use hours instead as default unit ? it seems most people would think of this
parameter in hours/days.



itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestReplChangeManager.java
(line 56)
<https://reviews.apache.org/r/54826/#comment230765>

    with recent changes by Vaibhav to ProxyFileSystem, local files would also have checksums.
Can we switch to local files, so that test speed is better (local fs might be more reliable
than minidfs, as it has less parts to it!)



itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestReplChangeManager.java
(line 59)
<https://reviews.apache.org/r/54826/#comment230766>

    why is this needed ?



itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestReplChangeManager.java
(line 124)
<https://reviews.apache.org/r/54826/#comment230768>

    how about creating a method for this and calling -Partition part1 = createAndPartition("20160101")



itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestReplChangeManager.java
(line 241)
<https://reviews.apache.org/r/54826/#comment230772>

    can you add a comment like -
    // verify cm.recycle(Path) api moves file to cmroot dir



itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestReplChangeManager.java
(line 247)
<https://reviews.apache.org/r/54826/#comment230773>

    // verify cm.recycle(db, table) api moves file to cmroot dir



metastore/src/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java (line 151)
<https://reviews.apache.org/r/54826/#comment230782>

    should we call it getCksumString() to make it more easy to understand what is expected
?



metastore/src/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java (line 175)
<https://reviews.apache.org/r/54826/#comment230776>

    is there any generic way to find this from FileSystem api ? these configs are specific
to HDFS.
    But i guess we don't need this if we go with storing only the signature (see following
comment)



metastore/src/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java (line 182)
<https://reviews.apache.org/r/54826/#comment230780>

    as discussed offline, looks like we don't want to rely on the directory structure of table.
    This can cause problems in finding file in case of insert -> rename -> drop .
    The signature should get used as the 'key' for finding the file.



metastore/src/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java (line 205)
<https://reviews.apache.org/r/54826/#comment230774>

    can you add a log message to indicate that this has started. Since this would typically
be run only once in hour or so , INFO level should be good IMO



metastore/src/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java (line 225)
<https://reviews.apache.org/r/54826/#comment230775>

    can you add a debug level log message for when files get deleted ?


- Thejas Nair


On Dec. 16, 2016, 11:14 p.m., Daniel Dai wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54826/
> -----------------------------------------------------------
> 
> (Updated Dec. 16, 2016, 11:14 p.m.)
> 
> 
> Review request for hive and Thejas Nair.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> See HIVE-15448
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 9064e49 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestReplChangeManager.java
PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java f7b2ed7 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java PRE-CREATION

>   metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java 6aca1b7 
> 
> Diff: https://reviews.apache.org/r/54826/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Daniel Dai
> 
>


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