----------------------------------------------------------- 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 > >