----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36512/#review141139 -----------------------------------------------------------
client/src/main/java/org/apache/oozie/client/OozieClient.java (line 2005) <https://reviews.apache.org/r/36512/#comment206573> It would be nice to make the Health Check also available from the CLI/client. I added the missing CLI commands a little while ago (get configuration, metrics/instrumentation, etc) so it would be nice to keep feature parity between the CLI/client and the REST API. core/src/main/java/org/apache/oozie/healthcheck/DataBaseHealthCheck.java (line 33) <https://reviews.apache.org/r/36512/#comment206560> Should we query on the OOZIE_SYS table? That way this shouldn't put any unnecessary pressure on the bundle table (or am I making that up? I'm hardly a database expert). core/src/main/java/org/apache/oozie/healthcheck/HdfsHealthCheck.java (line 36) <https://reviews.apache.org/r/36512/#comment206563> elsewhere, we had oozie.healthCheck.components. I don't care too much about which captialization we use, as long as we're consistent. core/src/main/java/org/apache/oozie/healthcheck/HdfsHealthCheck.java (line 56) <https://reviews.apache.org/r/36512/#comment206561> This can be moved outside of the for loop core/src/main/java/org/apache/oozie/healthcheck/HdfsHealthCheck.java (line 60) <https://reviews.apache.org/r/36512/#comment206562> Why not use has.createFileSystem(...) here? core/src/main/java/org/apache/oozie/healthcheck/HealthCheckExecutorHTML.java (line 28) <https://reviews.apache.org/r/36512/#comment206564> TODO core/src/main/java/org/apache/oozie/healthcheck/SharelibHealthCheck.java (lines 34 - 35) <https://reviews.apache.org/r/36512/#comment206565> It would be nice if we could also check that the sharelib is the correct version. While we can't verify that say, pig, is correct, we can check one of the oozie jars (e.g. oozie-sharelib-oozie-<version>.jar). Cloudera Manager actually has a check that does something just like this. It also looks like you did something like this for the Hadoop version in SharelibMetaFileHealthCheck. core/src/test/java/org/apache/oozie/client/TestOozieCLI.java (line 1312) <https://reviews.apache.org/r/36512/#comment206566> -1? Doesn't that mean that the command failed? core/src/test/java/org/apache/oozie/healthcheck/TestDataBaseHealthCheck.java (line 34) <https://reviews.apache.org/r/36512/#comment206567> ```` if (Services.get() != null) { Services.get().destroy(); } ```` Just in case. docs/src/site/twiki/AG_Install.twiki (lines 1021 - 1022) <https://reviews.apache.org/r/36512/#comment206568> I'd just omit the <description> tags here. docs/src/site/twiki/AG_Install.twiki (lines 1027 - 1031) <https://reviews.apache.org/r/36512/#comment206569> We should give a description of each of these, even if it should be mostly obvious from their names. Particularly, for the two sharelib ones, which may be confusing for users. docs/src/site/twiki/AG_Install.twiki (line 1032) <https://reviews.apache.org/r/36512/#comment206570> It might be good to mention that users can implement their own health checkers and give the name of the parent class to implement. docs/src/site/twiki/DG_CommandLineTool.twiki (line 1488) <https://reviews.apache.org/r/36512/#comment206572> The equivalent section is missing from the WebServicesAPI.twiki docs/src/site/twiki/DG_CommandLineTool.twiki (line 1490) <https://reviews.apache.org/r/36512/#comment206571> What about the ignoremissingtag argument? - Robert Kanter On July 6, 2016, 9:20 p.m., Purshotam Shah wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36512/ > ----------------------------------------------------------- > > (Updated July 6, 2016, 9:20 p.m.) > > > Review request for oozie. > > > Bugs: OOZIE-2306 > https://issues.apache.org/jira/browse/OOZIE-2306 > > > Repository: oozie-git > > > Description > ------- > > Oozie can be configured to check the health of its components. > http://localhost:11000/oozie/v2/admin/health will report the health of > configured components and time taken to compute health check (this can be > used to find component slowness). > Health check component can be configured with below properties. > <verbatim> > <property> > <name>oozie.healthCheck.components</name> > <value>org.apache.oozie.healthcheck.DataBaseHealthCheck</value> > <description> > </description> > </property> > </verbatim> > Support components are DataBaseHealthCheck, HdfsHealthCheck, > SharelibHealthCheck, SharelibMetaFileHealthCheck and ZKHealthCheck. > > > Diffs > ----- > > client/src/main/java/org/apache/oozie/cli/OozieCLI.java > 65291ff1303cb0e8419e2930521d26c0b3cd7142 > client/src/main/java/org/apache/oozie/client/OozieClient.java > 67a62c622a9ef86afb82b492735d146faba427de > client/src/main/java/org/apache/oozie/client/rest/JsonTags.java > b3b148acb8e0b71b6233937b793453009a4faf2a > client/src/main/java/org/apache/oozie/client/rest/RestConstants.java > 3c6931915a1afb3ead4b12e1b9ea3c02835c0b5d > core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java > 4fc0c52396165ea9e8add98368e360e3ec989b77 > core/src/main/java/org/apache/oozie/healthcheck/DataBaseHealthCheck.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > core/src/main/java/org/apache/oozie/healthcheck/HdfsHealthCheck.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > core/src/main/java/org/apache/oozie/healthcheck/HealthCheckExecutor.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > > core/src/main/java/org/apache/oozie/healthcheck/HealthCheckExecutorHTML.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > > core/src/main/java/org/apache/oozie/healthcheck/HealthCheckExecutorJson.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > core/src/main/java/org/apache/oozie/healthcheck/HealthCheckStatus.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > > core/src/main/java/org/apache/oozie/healthcheck/OozieHealthCheckComponent.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > core/src/main/java/org/apache/oozie/healthcheck/SharelibHealthCheck.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > > core/src/main/java/org/apache/oozie/healthcheck/SharelibMetaFileHealthCheck.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > core/src/main/java/org/apache/oozie/healthcheck/ZKHealthCheck.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > core/src/main/java/org/apache/oozie/service/ShareLibService.java > 81a9f2d5da781ff79b9bcb1cb5cca02575d57b49 > core/src/main/java/org/apache/oozie/servlet/BaseAdminServlet.java > 64d3f1f40c5c7b3746ad410cf43f80925424d094 > core/src/main/java/org/apache/oozie/servlet/JsonRestServlet.java > 059d3cf6dc251b49940af29d82cbdd817043a176 > core/src/main/java/org/apache/oozie/servlet/V0AdminServlet.java > fd573d51b77d86c77e944b4d49e1f77f30eb6a26 > core/src/main/java/org/apache/oozie/servlet/V1AdminServlet.java > 965a19a2f14e4ba10e5977bf042a2d3c2e47d525 > core/src/main/java/org/apache/oozie/servlet/V2AdminServlet.java > 7eadbe781e58f5bb674930288416ac4e7f0c3ae0 > core/src/main/resources/oozie-default.xml > 3ff7320df003a0ee05f767ffcf3d9a2b0b448d07 > core/src/test/java/org/apache/oozie/client/TestOozieCLI.java > 54bfc16131ed20218ef56e028a69a5507432bd1a > > core/src/test/java/org/apache/oozie/healthcheck/TestDataBaseHealthCheck.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > > core/src/test/java/org/apache/oozie/healthcheck/TestSharelibMetaFileHealthCheck.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > core/src/test/java/org/apache/oozie/healthcheck/TestZKHealthCheck.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > core/src/test/java/org/apache/oozie/service/ShareLibServiceTestCases.java > e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > core/src/test/java/org/apache/oozie/service/TestShareLibService.java > 842b2d53ed1f996f694c8960384e8455a9923372 > docs/src/site/twiki/AG_Install.twiki > 66c00199a3ef457cd495ec8b9717823b820d958a > docs/src/site/twiki/DG_CommandLineTool.twiki > ff1cce532efc3eb3b6c7e4e53f6db6b3ef26d718 > docs/src/site/twiki/WebServicesAPI.twiki > a34f2d354e6f073a6f89d951af36cffb0f8dadd8 > > Diff: https://reviews.apache.org/r/36512/diff/ > > > Testing > ------- > > Added enough test cases and checked manually. > > > Thanks, > > Purshotam Shah > >
