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



core/src/main/java/org/apache/oozie/dependency/hcat/EhcacheHCatDependencyCache.java
<https://reviews.apache.org/r/18754/#comment76906>

    Missing implementations



core/src/main/java/org/apache/oozie/dependency/hcat/HCatDependencyCache.java
<https://reviews.apache.org/r/18754/#comment76908>

    dependencies



core/src/main/java/org/apache/oozie/dependency/hcat/SimpleHCatDependencyCache.java
<https://reviews.apache.org/r/18754/#comment76910>

    long currentTime = new Date().getTime();



core/src/main/java/org/apache/oozie/dependency/hcat/SimpleHCatDependencyCache.java
<https://reviews.apache.org/r/18754/#comment76909>

    for (String actionId : registeredCoordActionMap.keySet())



core/src/main/java/org/apache/oozie/dependency/hcat/SimpleHCatDependencyCache.java
<https://reviews.apache.org/r/18754/#comment76911>

    Store Date as Long in the map



core/src/main/java/org/apache/oozie/dependency/hcat/SimpleHCatDependencyCache.java
<https://reviews.apache.org/r/18754/#comment76912>

    removeNonWaitingCoordActions
    
    Move purgeMissingDependency() logic into PartitionDependencyManagerService 
and make it common for all Cache implementations. 
removeStaleCoordActions(Set<String> staleActions) can be implemented by each 
implementation. 
    
    You can have each implementation return the registered coord actions with 
getRegisteredCoordActions() or have registeredCoordActionMap in 
PartitionDependencyManagerService itself.



core/src/main/java/org/apache/oozie/dependency/hcat/SimpleHCatDependencyCache.java
<https://reviews.apache.org/r/18754/#comment76918>

    Get rid of all the List<> delete.. and use iterator and remove them then 
and there within the loop.



core/src/main/java/org/apache/oozie/dependency/hcat/SimpleHCatDependencyCache.java
<https://reviews.apache.org/r/18754/#comment76916>

    synchronized (partKeyPatterns) {



core/src/main/java/org/apache/oozie/dependency/hcat/SimpleHCatDependencyCache.java
<https://reviews.apache.org/r/18754/#comment76917>

    else {
      continue;
    }



core/src/main/java/org/apache/oozie/dependency/hcat/SimpleHCatDependencyCache.java
<https://reviews.apache.org/r/18754/#comment76914>

    There will be only one table and please keep removing and unregistering as 
you see. If you delay there might be a new action registered for that table in 
the mean time and you will go remove it after that.  i.e do it inside the loop 
itself using a iterator and iterator.remove. Same for every map you iterate



core/src/main/java/org/apache/oozie/dependency/hcat/SimpleHCatDependencyCache.java
<https://reviews.apache.org/r/18754/#comment76915>

    Method not required. You can get hcat uri from one of the WaitingAction 
removed.



core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java
<https://reviews.apache.org/r/18754/#comment76913>

    Get rid of this setting and check if HA is enabled. You can add a method 
isHighlyAvailable() to JobsConcurrencyService



core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java
<https://reviews.apache.org/r/18754/#comment76919>

    You can check for non-waiting actions here and just call 
removeNonWaitingActions on the cache.



core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java
<https://reviews.apache.org/r/18754/#comment76920>

    You can have the registeredCoordActionMap in this class itself and remove 
it from there.



core/src/test/java/org/apache/oozie/service/TestHAPartitionDependencyManagerService.java
<https://reviews.apache.org/r/18754/#comment76923>

    Why wait for 30 seconds?



core/src/test/java/org/apache/oozie/service/TestHAPartitionDependencyManagerService.java
<https://reviews.apache.org/r/18754/#comment76921>

    just return false or true and get rid of the flag variable. You only need a 
variable if there is only one return statement at the end



sharelib/hcatalog/src/main/java/org/apache/oozie/util/HCatURI.java
<https://reviews.apache.org/r/18754/#comment76924>

    Changes not required. Can be reverted


- Rohini Palaniswamy


On May 13, 2014, 11:14 p.m., Ryota Egashira wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18754/
> -----------------------------------------------------------
> 
> (Updated May 13, 2014, 11:14 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-1492
>     https://issues.apache.org/jira/browse/OOZIE-1492
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> https://issues.apache.org/jira/browse/OOZIE-1492
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/oozie/CoordinatorActionBean.java 03a7ed8 
>   
> core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java
>  2e5cd47 
>   
> core/src/main/java/org/apache/oozie/dependency/hcat/EhcacheHCatDependencyCache.java
>  6f127c4 
>   
> core/src/main/java/org/apache/oozie/dependency/hcat/HCatDependencyCache.java 
> df3afd3 
>   
> core/src/main/java/org/apache/oozie/dependency/hcat/SimpleHCatDependencyCache.java
>  e8e3ebc 
>   
> core/src/main/java/org/apache/oozie/executor/jpa/CoordActionQueryExecutor.java
>  f5304ca 
>   
> core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java
>  985dcab 
>   
> core/src/test/java/org/apache/oozie/command/coord/TestCoordPushDependencyCheckXCommand.java
>  da09727 
>   
> core/src/test/java/org/apache/oozie/service/TestHAPartitionDependencyManagerService.java
>  PRE-CREATION 
>   
> core/src/test/java/org/apache/oozie/service/TestPartitionDependencyManagerService.java
>  ef71fb0 
>   sharelib/hcatalog/src/main/java/org/apache/oozie/util/HCatURI.java d797f9b 
>   sharelib/hcatalog/src/test/java/org/apache/oozie/util/TestHCatURI.java 
> caa2e8c 
> 
> Diff: https://reviews.apache.org/r/18754/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ryota Egashira
> 
>

Reply via email to