Re: Review Request 62459: OOZIE-2296: Add an Oozie diagnostic bundle tool

[Attila Sasvari <as...@cloudera.com>]

> On Sept. 29, 2017, 8:17 a.m., András Piros wrote:
> > tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
> > Lines 97-98 (patched)
> > <https://reviews.apache.org/r/62459/diff/7/?file=1837523#file1837523line97>
> >
> >     Put `resolvedActionsDir` and `configEntryWriter` to fields, and you got two parameters less.

Resolved action dir varies from workflow action to workflow action, and right now DiagBundleWriter constructor uses it.


> On Sept. 29, 2017, 8:17 a.m., András Piros wrote:
> > tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
> > Lines 99-119 (patched)
> > <https://reviews.apache.org/r/62459/diff/7/?file=1837523#file1837523line99>
> >
> >     Using a `StringBuilder`, maybe?

It is a writer not where we write to.


> On Sept. 29, 2017, 8:17 a.m., András Piros wrote:
> > tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
> > Lines 276-302 (patched)
> > <https://reviews.apache.org/r/62459/diff/7/?file=1837523#file1837523line276>
> >
> >     `StringBuffer`?

We are writing to a writer (StringBuffer's thread-safety is not needed).


> On Sept. 29, 2017, 8:17 a.m., András Piros wrote:
> > tools/src/main/java/org/apache/oozie/tools/diag/BundleCollectorDriver.java
> > Lines 43-45 (patched)
> > <https://reviews.apache.org/r/62459/diff/7/?file=1837525#file1837525line43>
> >
> >     `int`

Findbugs would complain about boxing / unboxing.


> On Sept. 29, 2017, 8:17 a.m., András Piros wrote:
> > tools/src/main/java/org/apache/oozie/tools/diag/BundleEntryWriter.java
> > Lines 85 (patched)
> > <https://reviews.apache.org/r/62459/diff/7/?file=1837527#file1837527line85>
> >
> >     What about try-with-resources?

Please expand. The writer object is open for a period time and write is called multiple times.


> On Sept. 29, 2017, 8:17 a.m., András Piros wrote:
> > tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java
> > Lines 58-64 (patched)
> > <https://reviews.apache.org/r/62459/diff/7/?file=1837529#file1837529line58>
> >
> >     `writeSection()`

You cannot easily reuse such function in this class, look at function calls and types.


- Attila


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


On Sept. 27, 2017, 2:12 p.m., Attila Sasvari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62459/
> -----------------------------------------------------------
> 
> (Updated Sept. 27, 2017, 2:12 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> A diagnostic tool that collects a bunch of job and other information from Oozie in a zip file.
> 
> 
> Diffs
> -----
> 
>   docs/src/site/twiki/DG_CommandLineTool.twiki d4047671876dcc3279a2ec379bc1d003f5e6f1aa 
>   pom.xml efccc346932514ada578a3462eb3c3cfe519a323 
>   tools/pom.xml 7306a14e7b237977be00f8fe28e34573540fd508 
>   tools/src/main/bin/oozie-diag-bundle-collector.sh PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/BundleCollectorDriver.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/BundleCompressor.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/BundleEntryWriter.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/Client.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java PRE-CREATION 
>   tools/src/test/java/org/apache/oozie/tools/diag/TestAppInfoCollector.java PRE-CREATION 
>   tools/src/test/java/org/apache/oozie/tools/diag/TestArgParser.java PRE-CREATION 
>   tools/src/test/java/org/apache/oozie/tools/diag/TestMetricsCollector.java PRE-CREATION 
>   tools/src/test/java/org/apache/oozie/tools/diag/TestServerInfoCollector.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62459/diff/7/
> 
> 
> Testing
> -------
> 
> - new unit tests: TestOozieDiagBundleCollector
> - started Oozie with a pseudo hadoop cluster, submitted a couple workflows, and executed the following commands: 
> -- ``bin/oozie-diag-bundle-collector.sh`` (usage info printed),
> -- ``bin/oozie-diag-bundle-collector.sh  -numworkflows 2000 -oozie http://localhost:11000/oozie -output /tmp``, 
> -- ``bin/oozie-diag-bundle-collector.sh  -jobs 0000001-170918144116149-oozie-asas-W -oozie http://localhost:11000/oozie -output .`` (verified zip the tool generated).
> 
> 
> Thanks,
> 
> Attila Sasvari
> 
>