> On Jan. 17, 2013, 12:01 a.m., Mona Chitnis wrote: > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java, > > line 103 > > <https://reviews.apache.org/r/8960/diff/1/?file=248546#file248546line103> > > > > add square brackets [] for actionId in log here
Will do > On Jan. 17, 2013, 12:01 a.m., Mona Chitnis wrote: > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/dependency/HCatURIHandler.java, > > line 200 > > <https://reviews.apache.org/r/8960/diff/1/?file=248549#file248549line200> > > > > this does not look formatted correctly. usually there is space between > > the Exception and the starting brace {. Check formatter being applied > > everywhere else Will do > On Jan. 17, 2013, 12:01 a.m., Mona Chitnis wrote: > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/dependency/cache/SimpleHCatDependencyCache.java, > > line 137 > > <https://reviews.apache.org/r/8960/diff/1/?file=248552#file248552line137> > > > > null check for sortedPKV.getPartKeys() and getPartVals() Not required. Will never be null. > On Jan. 17, 2013, 12:01 a.m., Mona Chitnis wrote: > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/dependency/cache/SimpleHCatDependencyCache.java, > > line 154 > > <https://reviews.apache.org/r/8960/diff/1/?file=248552#file248552line154> > > > > why this check? I thought this function returns all actions > > corresponding to that partition Need it, as we have to match the order of the partitions. > On Jan. 17, 2013, 12:01 a.m., Mona Chitnis wrote: > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/dependency/cache/WaitingAction.java, > > line 23 > > <https://reviews.apache.org/r/8960/diff/1/?file=248553#file248553line23> > > > > is this a combined list of all the dependency URIs of this actions? > > > > Also wasnt the Recovery Service expected to need the field 'Last > > Modified time' to batch-poll for stale untouched actions in cache? No. Recovery service was not using it. It was using the lastModifiedTime of the action. So removed it. > On Jan. 17, 2013, 12:01 a.m., Mona Chitnis wrote: > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java, > > line 146 > > <https://reviews.apache.org/r/8960/diff/1/?file=248557#file248557line146> > > > > remove from connectionsMap too? There will be other topics being listening in that connection. This just closes the session for a topic. Right now we are not closing the connection in the code. > On Jan. 17, 2013, 12:01 a.m., Mona Chitnis wrote: > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestPartitionDependencyManagerService.java, > > line 255 > > <https://reviews.apache.org/r/8960/diff/1/?file=248575#file248575line255> > > > > I think it is better to have separate methods to isolate the logic > > pieces of addition, removal, available etc Was testing the whole flow. To do removal, you need to do add first. Same for available. So avoided duplication. - Rohini ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8960/#review15417 ----------------------------------------------------------- On Jan. 15, 2013, 5:53 p.m., Rohini Palaniswamy wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8960/ > ----------------------------------------------------------- > > (Updated Jan. 15, 2013, 5:53 p.m.) > > > Review request for oozie. > > > Description > ------- > > 1) Changed the dependency check logic to include subset of partitions > 2) Extracted out the dependency cache and made an interface, so that a cache > which spills to disk can later be substituted. > 3) Some cleanup, fixes and optimizations > > > This addresses bug OOZIE-1167. > https://issues.apache.org/jira/browse/OOZIE-1167 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/client/src/main/java/org/apache/oozie/cli/OozieCLI.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/ErrorCode.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/dependency/DependencyChecker.java > PRE-CREATION > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/dependency/FSURIHandler.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/dependency/HCatURIHandler.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/dependency/URIHandler.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/dependency/cache/HCatDependencyCache.java > PRE-CREATION > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/dependency/cache/SimpleHCatDependencyCache.java > PRE-CREATION > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/dependency/cache/WaitingAction.java > PRE-CREATION > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/HCatMessageHandler.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/MessageHandler.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/MessageReceiver.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/service/MetadataServiceException.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/service/RecoveryService.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/service/URIHandlerService.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/util/HCatURI.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/util/PartitionWrapper.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/util/PartitionsGroup.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/util/WaitingActions.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/resources/oozie-default.xml > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionInputCheckXCommand.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionUpdatePushMissingDependency.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordPushDependencyCheckXCommand.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/coord/TestCoordCommandUtils.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestHCatMessageHandler.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestMessageReceiver.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestJMSAccessorService.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestPartitionDependencyManagerService.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestRecoveryService.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/test/MiniHCatServer.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/test/XHCatTestCase.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/test/XTestCase.java > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/resources/coord-job-for-matd-hcat.xml > 1431712 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/examples/src/test/java/org/apache/oozie/example/TestLocalOozieExample.java > 1431712 > > Diff: https://reviews.apache.org/r/8960/diff/ > > > Testing > ------- > > > Thanks, > > Rohini Palaniswamy > >
