----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26826/#review64423 -----------------------------------------------------------
Looks good; mostly minor comments. client/src/main/java/org/apache/oozie/cli/OozieCLI.java <https://reviews.apache.org/r/26826/#comment107119> "show locks held by Oozie server(s)" client/src/test/java/org/apache/oozie/cli/TestHAJsonParser.java <https://reviews.apache.org/r/26826/#comment107121> Can you give this a better name? core/src/main/java/org/apache/oozie/ha/inputrequest/HARequest.java <https://reviews.apache.org/r/26826/#comment107134> Is the plan to (eventually) rewrite existing calls to use HARequest as well? If so, you can either do it as part of this patch or create another JIRA. core/src/main/java/org/apache/oozie/ha/inputrequest/LockDumpHARequest.java <https://reviews.apache.org/r/26826/#comment107137> Could this cause a ConcurrentModificationException in rare cases? If something else adds/removes a lock while addAll(...) is reading through the locks, I think it could happen. core/src/main/java/org/apache/oozie/service/MemoryLocksService.java <https://reviews.apache.org/r/26826/#comment107136> These aren't just locks for jobs, right? These are all locks? We should rename this to just getLocks()? core/src/main/java/org/apache/oozie/servlet/V1AdminServlet.java <https://reviews.apache.org/r/26826/#comment107126> Remove this :) core/src/main/java/org/apache/oozie/servlet/V1AdminServlet.java <https://reviews.apache.org/r/26826/#comment107127> v1 core/src/test/java/org/apache/oozie/servlet/TestLocksDump.java <https://reviews.apache.org/r/26826/#comment107128> We should also have a test for when using the in memory locks. I think everything in this file is using the zookeeper locks because it subclasses ZKXTestCase, which configures that. docs/src/site/twiki/DG_CommandLineTool.twiki <https://reviews.apache.org/r/26826/#comment107130> show locks held by Oozie server(s) docs/src/site/twiki/WebServicesAPI.twiki <https://reviews.apache.org/r/26826/#comment107132> pluralize "lock" docs/src/site/twiki/WebServicesAPI.twiki <https://reviews.apache.org/r/26826/#comment107133> Shouldn't this be v2 and in the v2 section of the API? v1 throws an Exception. - Robert Kanter On Oct. 16, 2014, 11:57 p.m., Purshotam Shah wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26826/ > ----------------------------------------------------------- > > (Updated Oct. 16, 2014, 11:57 p.m.) > > > Review request for oozie. > > > Bugs: OOZIE-1931 > https://issues.apache.org/jira/browse/OOZIE-1931 > > > Repository: oozie-git > > > Description > ------- > > OOZIE-1931 Admin command to print all locks held by server(s) > > > Diffs > ----- > > client/src/main/java/org/apache/oozie/cli/HAJsonParser.java e69de29 > client/src/main/java/org/apache/oozie/cli/OozieCLI.java f3ffd1f > client/src/main/java/org/apache/oozie/client/OozieClient.java d6ff2d0 > client/src/main/java/org/apache/oozie/client/rest/JsonTags.java 8a86bf1 > client/src/main/java/org/apache/oozie/client/rest/RestConstants.java > 4b393c8 > client/src/test/java/org/apache/oozie/cli/TestHAJsonParser.java e69de29 > core/src/main/java/org/apache/oozie/ha/inputrequest/HARequest.java e69de29 > core/src/main/java/org/apache/oozie/ha/inputrequest/LockDumpHARequest.java > e69de29 > core/src/main/java/org/apache/oozie/lock/MemoryLocks.java ee564b3 > core/src/main/java/org/apache/oozie/service/MemoryLocksService.java e3eccdb > core/src/main/java/org/apache/oozie/service/ZKLocksService.java 36c00b9 > core/src/main/java/org/apache/oozie/servlet/BaseAdminServlet.java 64d3f1f > core/src/main/java/org/apache/oozie/servlet/JsonRestServlet.java 4e9c224 > core/src/main/java/org/apache/oozie/servlet/V0AdminServlet.java fd573d5 > core/src/main/java/org/apache/oozie/servlet/V1AdminServlet.java 2383433 > core/src/main/java/org/apache/oozie/servlet/V2AdminServlet.java adaab76 > core/src/test/java/org/apache/oozie/ha/inputrequest/TestHAVersion.java > e69de29 > core/src/test/java/org/apache/oozie/service/DummyV2AdminServlet.java > 0e466fc > core/src/test/java/org/apache/oozie/service/TestHAShareLibService.java > d2ad881 > core/src/test/java/org/apache/oozie/servlet/TestLocksDump.java e69de29 > docs/src/site/twiki/DG_CommandLineTool.twiki c65acbd > docs/src/site/twiki/WebServicesAPI.twiki 6f31240 > > Diff: https://reviews.apache.org/r/26826/diff/ > > > Testing > ------- > > UTC and manual testing > > > Thanks, > > Purshotam Shah > >
