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




ql/src/java/org/apache/hadoop/hive/ql/parse/repl/CopyUtils.java
Lines 51 (patched)
<https://reviews.apache.org/r/62360/#comment261959>

    This can be private



ql/src/java/org/apache/hadoop/hive/ql/parse/repl/CopyUtils.java
Lines 86 (patched)
<https://reviews.apache.org/r/62360/#comment261960>

    we can start with
    
    if(!srcFile.isUseSourcePath() && 
sourceFs.exists(srcFile.getEffectivePath())){continue;} 
    
    as the first statement inside the for loop for srcFiles, that way it 
wouldnt have to try and get checksum etc for files in CM. Check to see 
existence of CM is to make sure that while doing the copy the cleaner thread 
did not clean the CM.
    
    This will also make a few subsequent conditional checking irrelevant.



ql/src/java/org/apache/hadoop/hive/ql/parse/repl/CopyUtils.java
Lines 120 (patched)
<https://reviews.apache.org/r/62360/#comment261962>

    May be i am missing something but even when we are using the CM path in 
copyAndVerify towards the last, the file from CM itself might be deleted via 
the CM cleaner thread, in that case the doCopy will quietly fail and move on 
rather than failing replication.



ql/src/java/org/apache/hadoop/hive/ql/parse/repl/CopyUtils.java
Lines 130 (patched)
<https://reviews.apache.org/r/62360/#comment261961>

    copyAsUser and hiveConf are member variables, they dont need to be passed 
to functions.


- anishek


On Sept. 19, 2017, 12:14 a.m., Daniel Dai wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62360/
> -----------------------------------------------------------
> 
> (Updated Sept. 19, 2017, 12:14 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> See HIVE-16898
> 
> 
> Diffs
> -----
> 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java 
> 88d6a7a 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ReplCopyTask.java 54746d3 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/repl/CopyUtils.java 28e7bcb 
> 
> 
> Diff: https://reviews.apache.org/r/62360/diff/3/
> 
> 
> Testing
> -------
> 
> Manually test it with debugger: setup a breakpoint right before copy, and 
> drop table in another session.
> 
> 
> Thanks,
> 
> Daniel Dai
> 
>

Reply via email to