You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Attila Sasvari <as...@cloudera.com> on 2017/09/21 10:31:25 UTC

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

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

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 
  tools/src/main/bin/oozie-diag-bundle-collector.sh PRE-CREATION 
  tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/TestOozieDiagBundleCollector.java PRE-CREATION 


Diff: https://reviews.apache.org/r/62459/diff/1/


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


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

Posted by Attila Sasvari <as...@cloudera.com>.

> On Sept. 22, 2017, 8:38 a.m., Peter Cseh wrote:
> > tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
> > Lines 602-606 (patched)
> > <https://reviews.apache.org/r/62459/diff/2/?file=1832297#file1832297line602>
> >
> >     Please get the logs for the LauncherMapper based on externalID as well.

There is no LauncherMapper anymore, but I will get logs for the Oozie launcher job if possible.


- Attila


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


On Sept. 21, 2017, 5:10 p.m., Attila Sasvari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62459/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2017, 5:10 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/OozieDiagBundleCollector.java PRE-CREATION 
>   tools/src/test/java/org/apache/oozie/tools/TestOozieDiagBundleCollector.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62459/diff/2/
> 
> 
> 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
> 
>


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

Posted by Peter Cseh <ge...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62459/#review185956
-----------------------------------------------------------




tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 602-606 (patched)
<https://reviews.apache.org/r/62459/#comment262301>

    Please get the logs for the LauncherMapper based on externalID as well.


- Peter Cseh


On Sept. 21, 2017, 5:10 p.m., Attila Sasvari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62459/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2017, 5:10 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/OozieDiagBundleCollector.java PRE-CREATION 
>   tools/src/test/java/org/apache/oozie/tools/TestOozieDiagBundleCollector.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62459/diff/2/
> 
> 
> 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
> 
>


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

Posted by Peter Bacsko <pb...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62459/#review186053
-----------------------------------------------------------



My overall impression:

1. The class is too big, almost 1000 lines. Would it be possible to break it up somehow? I think the most obvious way to do is put different collector logics into different classes.
2. Code is strongly coupled with I/O operations, making it hard to test. Certain stuff, like filesystem operations (exists/mkdir) and stream creation should be wrapped and injected. See LocalFsOperations in oozie-sharelib. That way you can avoid direct IO. A mock factory for creating streams makes testing extremely easy because it can just return a StringWriter which can be examined in the tests.

- Peter Bacsko


On szept. 22, 2017, 12:50 du, Attila Sasvari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62459/
> -----------------------------------------------------------
> 
> (Updated szept. 22, 2017, 12:50 du)
> 
> 
> 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/OozieDiagBundleCollector.java PRE-CREATION 
>   tools/src/test/java/org/apache/oozie/tools/TestOozieDiagBundleCollector.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62459/diff/3/
> 
> 
> 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
> 
>


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

Posted by Attila Sasvari <as...@cloudera.com>.

> On Sept. 23, 2017, 1:18 p.m., Peter Bacsko wrote:
> > tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
> > Lines 250 (patched)
> > <https://reviews.apache.org/r/62459/diff/3/?file=1832850#file1832850line250>
> >
> >     Do we need Integer instead of int?

Findbugs found an issue about boxing/unboxing that is why it returns an Integer.


> On Sept. 23, 2017, 1:18 p.m., Peter Bacsko wrote:
> > tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
> > Lines 258 (patched)
> > <https://reviews.apache.org/r/62459/diff/3/?file=1832850#file1832850line258>
> >
> >     Do we need Integer instead of int?

See my reply above


- Attila


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


On Sept. 22, 2017, 12:50 p.m., Attila Sasvari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62459/
> -----------------------------------------------------------
> 
> (Updated Sept. 22, 2017, 12:50 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/OozieDiagBundleCollector.java PRE-CREATION 
>   tools/src/test/java/org/apache/oozie/tools/TestOozieDiagBundleCollector.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62459/diff/3/
> 
> 
> 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
> 
>


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

Posted by Peter Bacsko <pb...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62459/#review186052
-----------------------------------------------------------




tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 155 (patched)
<https://reviews.apache.org/r/62459/#comment262433>

    Synchronized?



tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 168 (patched)
<https://reviews.apache.org/r/62459/#comment262435>

    Misplaced brace



tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 238 (patched)
<https://reviews.apache.org/r/62459/#comment262436>

    Do we need Integer instead of int?



tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 250 (patched)
<https://reviews.apache.org/r/62459/#comment262437>

    Do we need Integer instead of int?



tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 258 (patched)
<https://reviews.apache.org/r/62459/#comment262438>

    Do we need Integer instead of int?



tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 297 (patched)
<https://reviews.apache.org/r/62459/#comment262439>

    We shouldn't just print the exception IMO. At least we log something, re-throw it, something that is more appropriate.



tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 318 (patched)
<https://reviews.apache.org/r/62459/#comment262440>

    See my comment above about exception handling



tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 337 (patched)
<https://reviews.apache.org/r/62459/#comment262441>

    See my comment above about exception handling



tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 356 (patched)
<https://reviews.apache.org/r/62459/#comment262442>

    See my comment above about exception handling



tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 407 (patched)
<https://reviews.apache.org/r/62459/#comment262443>

    See my comment above about exception handling



tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 463 (patched)
<https://reviews.apache.org/r/62459/#comment262444>

    See my comment above about exception handling



tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 483 (patched)
<https://reviews.apache.org/r/62459/#comment262445>

    See my comment above about exception handling



tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 508 (patched)
<https://reviews.apache.org/r/62459/#comment262446>

    See my comment above about exception handling



tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 523 (patched)
<https://reviews.apache.org/r/62459/#comment262447>

    See my comment above about exception handling



tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 538 (patched)
<https://reviews.apache.org/r/62459/#comment262448>

    See my comment above about exception handling



tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 548 (patched)
<https://reviews.apache.org/r/62459/#comment262457>

    There are different places in the code where the job type is checked. I believe these should be some mini helper methods and invoke them if necessary to determine the type of a job.
    
    Something like
    
    private boolean isWorkflow(String id) {
      return id.endsWith("-W");
    }



tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 578 (patched)
<https://reviews.apache.org/r/62459/#comment262449>

    Besides printing the stack trace, add some error message too



tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 677 (patched)
<https://reviews.apache.org/r/62459/#comment262450>

    See my comment above about exception handling



tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 685 (patched)
<https://reviews.apache.org/r/62459/#comment262456>

    Extract default encoding to a final static variable



tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 788 (patched)
<https://reviews.apache.org/r/62459/#comment262451>

    See my comment above about exception handling



tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 837 (patched)
<https://reviews.apache.org/r/62459/#comment262452>

    See my comment above about exception handling



tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 872 (patched)
<https://reviews.apache.org/r/62459/#comment262454>

    Extract default encoding to a final static variable



tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 878 (patched)
<https://reviews.apache.org/r/62459/#comment262453>

    See my comment above about exception handling



tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 883 (patched)
<https://reviews.apache.org/r/62459/#comment262455>

    Extract default encoding to a final static variable


- Peter Bacsko


On szept. 22, 2017, 12:50 du, Attila Sasvari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62459/
> -----------------------------------------------------------
> 
> (Updated szept. 22, 2017, 12:50 du)
> 
> 
> 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/OozieDiagBundleCollector.java PRE-CREATION 
>   tools/src/test/java/org/apache/oozie/tools/TestOozieDiagBundleCollector.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62459/diff/3/
> 
> 
> 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
> 
>


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

Posted by Peter Bacsko <pb...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62459/#review186059
-----------------------------------------------------------




tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 237 (patched)
<https://reviews.apache.org/r/62459/#comment262469>

    We have to re-think what and how we test. What's the rationale behind selecting these four methods? With proper unit testing validating the negative behaviour should be straightforward and these methods could be private.



tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 911 (patched)
<https://reviews.apache.org/r/62459/#comment262466>

    This annotation looks unnecessary


- Peter Bacsko


On szept. 22, 2017, 12:50 du, Attila Sasvari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62459/
> -----------------------------------------------------------
> 
> (Updated szept. 22, 2017, 12:50 du)
> 
> 
> 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/OozieDiagBundleCollector.java PRE-CREATION 
>   tools/src/test/java/org/apache/oozie/tools/TestOozieDiagBundleCollector.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62459/diff/3/
> 
> 
> 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
> 
>


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

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


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

Posted by Peter Cseh <ge...@cloudera.com>.

> On Sept. 29, 2017, 8:17 a.m., András Piros wrote:
> > tools/src/main/bin/oozie-diag-bundle-collector.sh
> > Lines 55-58 (patched)
> > <https://reviews.apache.org/r/62459/diff/7/?file=1837522#file1837522line55>
> >
> >     Extract function.

I don't like the idea of extracting all these functions in a shell script. None of the extracted functions would deduplicate code and none of the will be used in other shell scripts either. I find it way easier just to read the shell script as it is rather that following function calls in it.


- Peter


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


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

Posted by András Piros <an...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62459/#review186568
-----------------------------------------------------------




tools/src/main/bin/oozie-diag-bundle-collector.sh
Lines 23-31 (patched)
<https://reviews.apache.org/r/62459/#comment263125>

    Extract function.



tools/src/main/bin/oozie-diag-bundle-collector.sh
Lines 33-34 (patched)
<https://reviews.apache.org/r/62459/#comment263126>

    Extract function.



tools/src/main/bin/oozie-diag-bundle-collector.sh
Lines 36 (patched)
<https://reviews.apache.org/r/62459/#comment263121>

    `OOZIECP` or `OOZIECLASSPATH` would be a better name.



tools/src/main/bin/oozie-diag-bundle-collector.sh
Lines 37-45 (patched)
<https://reviews.apache.org/r/62459/#comment263122>

    Extract function.



tools/src/main/bin/oozie-diag-bundle-collector.sh
Lines 48-53 (patched)
<https://reviews.apache.org/r/62459/#comment263123>

    Extract function.



tools/src/main/bin/oozie-diag-bundle-collector.sh
Lines 55-58 (patched)
<https://reviews.apache.org/r/62459/#comment263124>

    Extract function.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 51 (patched)
<https://reviews.apache.org/r/62459/#comment263127>

    `MAX_TIMEOUT_SECONDS` would be a better name.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 60 (patched)
<https://reviews.apache.org/r/62459/#comment263141>

    `storeWorkflowJobDetails()` might be a better name.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 67-73 (patched)
<https://reviews.apache.org/r/62459/#comment263165>

    This is happening three times in code. Extract to one and reuse.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 69 (patched)
<https://reviews.apache.org/r/62459/#comment263135>

    Returning here and having no `else` path seems cleaner to me.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 71-77 (patched)
<https://reviews.apache.org/r/62459/#comment263142>

    Extract method.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 76 (patched)
<https://reviews.apache.org/r/62459/#comment263136>

    `"Could not create resolved actions directory:"`



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 81-83 (patched)
<https://reviews.apache.org/r/62459/#comment263137>

    Nice :)



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 88 (patched)
<https://reviews.apache.org/r/62459/#comment263138>

    `"Got details for ... successfully"`, maybe?



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 93 (patched)
<https://reviews.apache.org/r/62459/#comment263140>

    Maybe printing only the `Exception` message is a better idea.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 97-98 (patched)
<https://reviews.apache.org/r/62459/#comment263147>

    Put `resolvedActionsDir` and `configEntryWriter` to fields, and you got two parameters less.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 99-119 (patched)
<https://reviews.apache.org/r/62459/#comment263144>

    Using a `StringBuilder`, maybe?



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 118 (patched)
<https://reviews.apache.org/r/62459/#comment263143>

    `ACTIONS`



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 120 (patched)
<https://reviews.apache.org/r/62459/#comment263148>

    Put this just before `while`.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 124-127 (patched)
<https://reviews.apache.org/r/62459/#comment263145>

    Substitute w/ a normal `while` loop might be more readable.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 128-149 (patched)
<https://reviews.apache.org/r/62459/#comment263146>

    Using a `StringBuilder`, maybe?



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 158-160 (patched)
<https://reviews.apache.org/r/62459/#comment263149>

    `persistWorkflowAction()`



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 170 (patched)
<https://reviews.apache.org/r/62459/#comment263150>

    It could implement `Runnable`, in which case you don't need to `return null`.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 182 (patched)
<https://reviews.apache.org/r/62459/#comment263171>

    `StandardCharsets.UTF_8.toString()` seems cleaner to me.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 189 (patched)
<https://reviews.apache.org/r/62459/#comment263151>

    `IllegalStateException`



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 194 (patched)
<https://reviews.apache.org/r/62459/#comment263152>

    Print only the message.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 204 (patched)
<https://reviews.apache.org/r/62459/#comment263153>

    Firing up and shutting down an `ExecutorService` I'd put to methods / places that are called only once per JVM / component lifetime.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 214 (patched)
<https://reviews.apache.org/r/62459/#comment263177>

    Only message.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 217 (patched)
<https://reviews.apache.org/r/62459/#comment263154>

    Firing up and shutting down an `ExecutorService` I'd put to methods / places that are called only once per JVM / component lifetime.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 220-233 (patched)
<https://reviews.apache.org/r/62459/#comment263155>

    Extract to another (maybe nested) class.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 235 (patched)
<https://reviews.apache.org/r/62459/#comment263156>

    Putting `outputDir` to a field would result in one less parameter.
    
    `storeCoordinatorJobInfo()` would be a better name.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 242-249 (patched)
<https://reviews.apache.org/r/62459/#comment263164>

    This is happening three times in code. Extract to one and reuse.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 260-264 (patched)
<https://reviews.apache.org/r/62459/#comment263157>

    `while` loop instead.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 270 (patched)
<https://reviews.apache.org/r/62459/#comment263158>

    Print only the message.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 274 (patched)
<https://reviews.apache.org/r/62459/#comment263159>

    `configEntryWriter` could possibly be a field.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 276-302 (patched)
<https://reviews.apache.org/r/62459/#comment263160>

    `StringBuffer`?



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 304-308 (patched)
<https://reviews.apache.org/r/62459/#comment263161>

    `while` loop



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 309-324 (patched)
<https://reviews.apache.org/r/62459/#comment263162>

    `StringBuffer`?



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 336-343 (patched)
<https://reviews.apache.org/r/62459/#comment263163>

    This is happening three times in code. Extract to one and reuse.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 361 (patched)
<https://reviews.apache.org/r/62459/#comment263166>

    Print only the message.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 365 (patched)
<https://reviews.apache.org/r/62459/#comment263168>

    `configEntryWriter` as a field



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 366-380 (patched)
<https://reviews.apache.org/r/62459/#comment263167>

    Maybe `StringBuilder`?



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 383 (patched)
<https://reviews.apache.org/r/62459/#comment263169>

    `storeCommonDetails`



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 399 (patched)
<https://reviews.apache.org/r/62459/#comment263170>

    Print only the message.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 405 (patched)
<https://reviews.apache.org/r/62459/#comment263172>

    `StandardCharsets.UTF_8.toString()` seems cleaner to me.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 428 (patched)
<https://reviews.apache.org/r/62459/#comment263174>

    `storeLastWorkflows()`



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 440 (patched)
<https://reviews.apache.org/r/62459/#comment263175>

    Only message.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 456 (patched)
<https://reviews.apache.org/r/62459/#comment263176>

    Only message.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 472 (patched)
<https://reviews.apache.org/r/62459/#comment263178>

    Only message.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 492-502 (patched)
<https://reviews.apache.org/r/62459/#comment263134>

    Would extract to `ParentIdChecker`. Actually, sometimes the portion before `@` has to be checked, though.



tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java
Lines 37 (patched)
<https://reviews.apache.org/r/62459/#comment263129>

    Please remove newline.



tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java
Lines 53 (patched)
<https://reviews.apache.org/r/62459/#comment263190>

    `setupArgParseOptions`



tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java
Lines 54-57 (patched)
<https://reviews.apache.org/r/62459/#comment263191>

    `createAndAddOption()`



tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java
Lines 59-63 (patched)
<https://reviews.apache.org/r/62459/#comment263192>

    `createAndAddOption()`



tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java
Lines 65-69 (patched)
<https://reviews.apache.org/r/62459/#comment263193>

    `createAndAddOption()`



tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java
Lines 71-75 (patched)
<https://reviews.apache.org/r/62459/#comment263194>

    `createAndAddOption()`



tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java
Lines 77-82 (patched)
<https://reviews.apache.org/r/62459/#comment263195>

    `createAndAddOption()`



tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java
Lines 84-88 (patched)
<https://reviews.apache.org/r/62459/#comment263196>

    `createAndAddOption()`



tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java
Lines 90-93 (patched)
<https://reviews.apache.org/r/62459/#comment263197>

    `createAndAddOption()`



tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java
Lines 96-102 (patched)
<https://reviews.apache.org/r/62459/#comment263198>

    Covered by `createAndAddOption()` calls.



tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java
Lines 106 (patched)
<https://reviews.apache.org/r/62459/#comment263199>

    `ensureOutputDir()`



tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java
Lines 118-120 (patched)
<https://reviews.apache.org/r/62459/#comment263200>

    `int parseAndCheck()`



tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java
Lines 129-132 (patched)
<https://reviews.apache.org/r/62459/#comment263201>

    `int parseAndCheck()`



tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java
Lines 136-139 (patched)
<https://reviews.apache.org/r/62459/#comment263202>

    `int parseAndCheck()`



tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java
Lines 143-146 (patched)
<https://reviews.apache.org/r/62459/#comment263203>

    `int parseAndCheck()`



tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java
Lines 168-173 (patched)
<https://reviews.apache.org/r/62459/#comment263204>

    `handleParseError()`



tools/src/main/java/org/apache/oozie/tools/diag/BundleCollectorDriver.java
Lines 40-41 (patched)
<https://reviews.apache.org/r/62459/#comment263206>

    `Locale.getDefault()`? Is this (and need to be) thread safe?



tools/src/main/java/org/apache/oozie/tools/diag/BundleCollectorDriver.java
Lines 43-45 (patched)
<https://reviews.apache.org/r/62459/#comment263208>

    `int`



tools/src/main/java/org/apache/oozie/tools/diag/BundleCollectorDriver.java
Lines 47 (patched)
<https://reviews.apache.org/r/62459/#comment263210>

    `int`



tools/src/main/java/org/apache/oozie/tools/diag/BundleCollectorDriver.java
Lines 95 (patched)
<https://reviews.apache.org/r/62459/#comment263408>

    `collectAndStoreDiagInfo()`



tools/src/main/java/org/apache/oozie/tools/diag/BundleCollectorDriver.java
Lines 102-108 (patched)
<https://reviews.apache.org/r/62459/#comment263407>

    `collectAndStoreServerInfo()`



tools/src/main/java/org/apache/oozie/tools/diag/BundleCollectorDriver.java
Lines 110-112 (patched)
<https://reviews.apache.org/r/62459/#comment263409>

    `collectAndStoreMetricsInfo()`



tools/src/main/java/org/apache/oozie/tools/diag/BundleCollectorDriver.java
Lines 114-118 (patched)
<https://reviews.apache.org/r/62459/#comment263410>

    `collectAndStoreAppInfo()`



tools/src/main/java/org/apache/oozie/tools/diag/BundleCollectorDriver.java
Lines 121 (patched)
<https://reviews.apache.org/r/62459/#comment263212>

    `checkOozieConnection()`



tools/src/main/java/org/apache/oozie/tools/diag/BundleCollectorDriver.java
Lines 136-142 (patched)
<https://reviews.apache.org/r/62459/#comment263213>

    `initServices()`



tools/src/main/java/org/apache/oozie/tools/diag/BundleCollectorDriver.java
Lines 149 (patched)
<https://reviews.apache.org/r/62459/#comment263179>

    Only message.



tools/src/main/java/org/apache/oozie/tools/diag/BundleEntryWriter.java
Lines 39 (patched)
<https://reviews.apache.org/r/62459/#comment263173>

    `StandardCharsets.UTF_8.toString()` seems cleaner to me.



tools/src/main/java/org/apache/oozie/tools/diag/BundleEntryWriter.java
Lines 41 (patched)
<https://reviews.apache.org/r/62459/#comment263219>

    Nice :)



tools/src/main/java/org/apache/oozie/tools/diag/BundleEntryWriter.java
Lines 49 (patched)
<https://reviews.apache.org/r/62459/#comment263412>

    `nullPlaceholder`



tools/src/main/java/org/apache/oozie/tools/diag/BundleEntryWriter.java
Lines 53 (patched)
<https://reviews.apache.org/r/62459/#comment263222>

    `writeLong()`



tools/src/main/java/org/apache/oozie/tools/diag/BundleEntryWriter.java
Lines 57 (patched)
<https://reviews.apache.org/r/62459/#comment263223>

    `writeInt()`



tools/src/main/java/org/apache/oozie/tools/diag/BundleEntryWriter.java
Lines 61 (patched)
<https://reviews.apache.org/r/62459/#comment263224>

    `writeString()`



tools/src/main/java/org/apache/oozie/tools/diag/BundleEntryWriter.java
Lines 64 (patched)
<https://reviews.apache.org/r/62459/#comment263226>

    Extract variable `valuePlaceholder`.



tools/src/main/java/org/apache/oozie/tools/diag/BundleEntryWriter.java
Lines 66 (patched)
<https://reviews.apache.org/r/62459/#comment263227>

    Extract variable `nullPlaceholder`.



tools/src/main/java/org/apache/oozie/tools/diag/BundleEntryWriter.java
Lines 68 (patched)
<https://reviews.apache.org/r/62459/#comment263228>

    Extract variable `emptyPlaceholder`.



tools/src/main/java/org/apache/oozie/tools/diag/BundleEntryWriter.java
Lines 70 (patched)
<https://reviews.apache.org/r/62459/#comment263229>

    Extract variable `newline`.



tools/src/main/java/org/apache/oozie/tools/diag/BundleEntryWriter.java
Lines 83 (patched)
<https://reviews.apache.org/r/62459/#comment263180>

    Only message.



tools/src/main/java/org/apache/oozie/tools/diag/BundleEntryWriter.java
Lines 85 (patched)
<https://reviews.apache.org/r/62459/#comment263230>

    What about try-with-resources?



tools/src/main/java/org/apache/oozie/tools/diag/Client.java
Lines 33 (patched)
<https://reviews.apache.org/r/62459/#comment263130>

    `DiagnosticsClient` would be a better name.



tools/src/main/java/org/apache/oozie/tools/diag/Client.java
Lines 40 (patched)
<https://reviews.apache.org/r/62459/#comment263133>

    `createRetryingConnection()` would be a better name.



tools/src/main/java/org/apache/oozie/tools/diag/Client.java
Lines 42-43 (patched)
<https://reviews.apache.org/r/62459/#comment263131>

    What about try-with-resources?



tools/src/main/java/org/apache/oozie/tools/diag/Client.java
Lines 48-49 (patched)
<https://reviews.apache.org/r/62459/#comment263132>

    What about try-with-resources?



tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java
Lines 37 (patched)
<https://reviews.apache.org/r/62459/#comment263232>

    `collectAndStore()`



tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java
Lines 51-57 (patched)
<https://reviews.apache.org/r/62459/#comment263237>

    `writeSection()`



tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java
Lines 58-64 (patched)
<https://reviews.apache.org/r/62459/#comment263238>

    `writeSection()`



tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java
Lines 65-74 (patched)
<https://reviews.apache.org/r/62459/#comment263239>

    `writeMultilineSection()`



tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java
Lines 75-84 (patched)
<https://reviews.apache.org/r/62459/#comment263240>

    `writeMultilineSection()`



tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java
Lines 89 (patched)
<https://reviews.apache.org/r/62459/#comment263187>

    Only message.



tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java
Lines 93 (patched)
<https://reviews.apache.org/r/62459/#comment263242>

    `collectAndStoreInstrumentationInfo()`



tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java
Lines 106-112 (patched)
<https://reviews.apache.org/r/62459/#comment263243>

    `writeSection()`



tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java
Lines 113-119 (patched)
<https://reviews.apache.org/r/62459/#comment263244>

    `writeSection()`



tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java
Lines 120-126 (patched)
<https://reviews.apache.org/r/62459/#comment263245>

    `writeSection()`



tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java
Lines 127-136 (patched)
<https://reviews.apache.org/r/62459/#comment263246>

    `writeMultilineSection()`



tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java
Lines 141 (patched)
<https://reviews.apache.org/r/62459/#comment263189>

    Only message.



tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java
Lines 40 (patched)
<https://reviews.apache.org/r/62459/#comment263248>

    `collectAndStoreShareLibInfo()`



tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java
Lines 49-50 (patched)
<https://reviews.apache.org/r/62459/#comment263247>

    `while` might be better readable



tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java
Lines 59 (patched)
<https://reviews.apache.org/r/62459/#comment263181>

    Only message.



tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java
Lines 63 (patched)
<https://reviews.apache.org/r/62459/#comment263249>

    `collectAndStoreJavaSystemProperties()`



tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java
Lines 68-75 (patched)
<https://reviews.apache.org/r/62459/#comment263252>

    `storeOneLineEntries()`



tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java
Lines 79 (patched)
<https://reviews.apache.org/r/62459/#comment263182>

    Only message.



tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java
Lines 83 (patched)
<https://reviews.apache.org/r/62459/#comment263250>

    `collectAndStoreOsEnv()`



tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java
Lines 88-94 (patched)
<https://reviews.apache.org/r/62459/#comment263253>

    `storeOneLineEntries()`



tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java
Lines 99 (patched)
<https://reviews.apache.org/r/62459/#comment263188>

    Only message.



tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java
Lines 103 (patched)
<https://reviews.apache.org/r/62459/#comment263254>

    `collectAndStoreServerConfiguration()`



tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java
Lines 120 (patched)
<https://reviews.apache.org/r/62459/#comment263183>

    Only message.



tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java
Lines 124 (patched)
<https://reviews.apache.org/r/62459/#comment263255>

    `collectAndStoreThreadDump()`



tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java
Lines 131 (patched)
<https://reviews.apache.org/r/62459/#comment263184>

    Only message.



tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java
Lines 135 (patched)
<https://reviews.apache.org/r/62459/#comment263256>

    `collectAndStoreCallableQueueDump()`



tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java
Lines 138 (patched)
<https://reviews.apache.org/r/62459/#comment263258>

    `dumpLines`



tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java
Lines 143 (patched)
<https://reviews.apache.org/r/62459/#comment263257>

    `dumpLine`



tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java
Lines 152 (patched)
<https://reviews.apache.org/r/62459/#comment263185>

    Only message.



tools/src/test/java/org/apache/oozie/tools/diag/TestAppInfoCollector.java
Lines 53-66 (patched)
<https://reviews.apache.org/r/62459/#comment263259>

    `setupBasicFields()`



tools/src/test/java/org/apache/oozie/tools/diag/TestAppInfoCollector.java
Lines 68-76 (patched)
<https://reviews.apache.org/r/62459/#comment263260>

    `setupDateFields()`



tools/src/test/java/org/apache/oozie/tools/diag/TestAppInfoCollector.java
Lines 78-101 (patched)
<https://reviews.apache.org/r/62459/#comment263262>

    `setupActions()`



tools/src/test/java/org/apache/oozie/tools/diag/TestAppInfoCollector.java
Lines 115-118 (patched)
<https://reviews.apache.org/r/62459/#comment263265>

    `setupBasicCoordinatorFields()`



tools/src/test/java/org/apache/oozie/tools/diag/TestAppInfoCollector.java
Lines 120-124 (patched)
<https://reviews.apache.org/r/62459/#comment263266>

    `setupCoordinatorStatus()`



tools/src/test/java/org/apache/oozie/tools/diag/TestAppInfoCollector.java
Lines 138-141 (patched)
<https://reviews.apache.org/r/62459/#comment263413>

    `setupBundle()`



tools/src/test/java/org/apache/oozie/tools/diag/TestArgParser.java
Lines 38 (patched)
<https://reviews.apache.org/r/62459/#comment263414>

    `originalSecurityManager`, and not `static`



tools/src/test/java/org/apache/oozie/tools/diag/TestArgParser.java
Lines 45 (patched)
<https://reviews.apache.org/r/62459/#comment263415>

    `interceptSystemSecurityManager()`



tools/src/test/java/org/apache/oozie/tools/diag/TestArgParser.java
Lines 65-66 (patched)
<https://reviews.apache.org/r/62459/#comment263416>

    `interceptSystemStreams()`
    
    When do we plan setting back the original ones?



tools/src/test/java/org/apache/oozie/tools/diag/TestMetricsCollector.java
Lines 115 (patched)
<https://reviews.apache.org/r/62459/#comment263418>

    woo-hoo :D



tools/src/test/java/org/apache/oozie/tools/diag/TestMetricsCollector.java
Lines 118 (patched)
<https://reviews.apache.org/r/62459/#comment263417>

    Shadowing field `testOut`.



tools/src/test/java/org/apache/oozie/tools/diag/TestMetricsCollector.java
Lines 122-127 (patched)
<https://reviews.apache.org/r/62459/#comment263419>

    Guava's `CharStreams#toString()` to the rescue.



tools/src/test/java/org/apache/oozie/tools/diag/TestMetricsCollector.java
Lines 216-221 (patched)
<https://reviews.apache.org/r/62459/#comment263420>

    Guava's `CharStreams#toString()` to the rescue.



tools/src/test/java/org/apache/oozie/tools/diag/TestMetricsCollector.java
Lines 232 (patched)
<https://reviews.apache.org/r/62459/#comment263186>

    Only message.



tools/src/test/java/org/apache/oozie/tools/diag/TestServerInfoCollector.java
Lines 102-106 (patched)
<https://reviews.apache.org/r/62459/#comment263421>

    Guava's `CharStreams#toString()` to the rescue.


- András Piros


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


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

Posted by Peter Cseh <ge...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62459/#review186564
-----------------------------------------------------------




tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java
Lines 36 (patched)
<https://reviews.apache.org/r/62459/#comment263120>

    Shouldn't this make the findbugs warnings go away?


- Peter Cseh


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


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

Posted by Peter Bacsko <pb...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62459/#review186940
-----------------------------------------------------------




tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 77 (patched)
<https://reviews.apache.org/r/62459/#comment263839>

    I think this should go to stdout instead



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 113 (patched)
<https://reviews.apache.org/r/62459/#comment263846>

    I see writeString("\n") at least a dozen of times (actually 18).
    
    I strongly suggest adding a method to the writer and call it writeNewLine().



tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleCollectorDriver.java
Lines 41 (patched)
<https://reviews.apache.org/r/62459/#comment263842>

    We might as we call this DATE_FORMAT. Also, if it's referenced from other classes (and I see it is), just make it public.



tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleCollectorDriver.java
Lines 47 (patched)
<https://reviews.apache.org/r/62459/#comment263843>

    I know that this is returned by the parser, but can we switch to List<String> here if it's not a big deal? It just feels safer.



tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleCompressor.java
Lines 31 (patched)
<https://reviews.apache.org/r/62459/#comment263847>

    Nit: I know that zip() is expressive in itself, but still, the name compress() sounds better to me.



tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleEntryWriter.java
Lines 48 (patched)
<https://reviews.apache.org/r/62459/#comment263840>

    This is usually thrown by the JVM when a static initializer block fails. Could we change this to something else? An ordinary RuntimeException looks good enough to me.



tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java
Lines 46 (patched)
<https://reviews.apache.org/r/62459/#comment263844>

    Nit: space :D



tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java
Lines 52 (patched)
<https://reviews.apache.org/r/62459/#comment263845>

    Do we need to call flush() here?



tools/src/test/java/org/apache/oozie/tools/diag/TestMetricsCollector.java
Lines 56 (patched)
<https://reviews.apache.org/r/62459/#comment263848>

    Could this whole example metrics be stored in a file and not inline in the code?



tools/src/test/java/org/apache/oozie/tools/diag/TestMetricsCollector.java
Lines 122 (patched)
<https://reviews.apache.org/r/62459/#comment263852>

    Simpler:
    
    import static java.nio.file.Files.readAllBytes;
    import static java.nio.file.Paths.get;
    
    String s = new String(readAllBytes(get("test.txt"))));
    
    This part should be extracted somewhere to avoid repeated coding to load a file



tools/src/test/java/org/apache/oozie/tools/diag/TestMetricsCollector.java
Lines 127 (patched)
<https://reviews.apache.org/r/62459/#comment263855>

    The string "UTF-8" is repeated a couple of times. Make it const or use StandardCharsets.UTF_8.



tools/src/test/java/org/apache/oozie/tools/diag/TestMetricsCollector.java
Lines 134 (patched)
<https://reviews.apache.org/r/62459/#comment263849>

    Same here, I would store this in an external file



tools/src/test/java/org/apache/oozie/tools/diag/TestMetricsCollector.java
Lines 215 (patched)
<https://reviews.apache.org/r/62459/#comment263853>

    Simplify file loading



tools/src/test/java/org/apache/oozie/tools/diag/TestMetricsCollector.java
Lines 231 (patched)
<https://reviews.apache.org/r/62459/#comment263850>

    Handle this catch properly



tools/src/test/java/org/apache/oozie/tools/diag/TestServerInfoCollector.java
Lines 120 (patched)
<https://reviews.apache.org/r/62459/#comment263854>

    Simplify file loading


- Peter Bacsko


On okt. 2, 2017, 9:39 de, Attila Sasvari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62459/
> -----------------------------------------------------------
> 
> (Updated okt. 2, 2017, 9:39 de)
> 
> 
> 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 0b94484da1c97618e9168cea0ebbfff7f70f723c 
>   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/DiagBundleCollectorDriver.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleCompressor.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleEntryWriter.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagOozieClient.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/9/
> 
> 
> 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
> 
>


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

Posted by Peter Cseh via Review Board <no...@reviews.apache.org>.

> On Oct. 6, 2017, 12:26 a.m., Robert Kanter wrote:
> > tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
> > Lines 173-174 (patched)
> > <https://reviews.apache.org/r/62459/diff/10/?file=1845962#file1845962line173>
> >
> >     ````LogAggregationUtils```` is marked ````@Private```` so we shouldn't be using it.  Hadoop can change things incompatibly here.
> 
> Attila Sasvari wrote:
>     What do you think about borrowing/inlining/copying those functions from Hadoop 2.6 too? I was thinking about it and it is probably better than bringing back the initial ``ExecutorService`` based approach. I am thinking of a class like:
>     
>     ```java
>     // TODO: once OOZIE-2983 ("Stream the Launcher AM Logs") is done, remove it.
>     public class OozieLauncherLogFetcher {
>         private static final String TMP_FILE_SUFFIX = ".tmp";
>         final private Configuration hadoopConfig;
>     
>         public OozieLauncherLogFetcher(final Configuration hadoopConfig) {
>             this.hadoopConfig = hadoopConfig;
>         }
>     
>         // Borrowed code from org.apache.hadoop.yarn.logaggregation.LogCLIHelpers
>         private static void logDirNotExist(String remoteAppLogDir) {
>             System.out.println(remoteAppLogDir + "does not exist.");
>             System.out.println("Log aggregation has not completed or is not enabled.");
>         }
>         // Borrowed code from org.apache.hadoop.yarn.logaggregation.LogCLIHelpers
>         private static void emptyLogDir(String remoteAppLogDir) {
>             System.out.println(remoteAppLogDir + "does not have any log files.");
>         }
>         // Borrowed code from org.apache.hadoop.yarn.logaggregation.LogAggregationUtils
>         public static String getRemoteNodeLogDirSuffix(Configuration conf) {
>             return conf.get(YarnConfiguration.NM_REMOTE_APP_LOG_DIR_SUFFIX, YarnConfiguration.DEFAULT_NM_REMOTE_APP_LOG_DIR_SUFFIX);
>         }
>         // Borrowed code from org.apache.hadoop.yarn.logaggregation.LogAggregationUtils
>         public static Path getRemoteLogSuffixedDir(Path remoteRootLogDir, String user, String suffix) {
>             return suffix != null && !suffix.isEmpty() ? new Path(getRemoteLogUserDir(remoteRootLogDir, user), suffix) :
>                     getRemoteLogUserDir(remoteRootLogDir, user);
>         }
>         // Borrowed code from org.apache.hadoop.yarn.logaggregation.LogAggregationUtils
>         public static Path getRemoteLogUserDir(Path remoteRootLogDir, String user) {
>             return new Path(remoteRootLogDir, user);
>         }
>     
>         // Borrowed code from org.apache.hadoop.yarn.logaggregation.LogAggregationUtils
>         public static Path getRemoteAppLogDir(Path remoteRootLogDir, ApplicationId appId, String user, String suffix) {
>             return new Path(getRemoteLogSuffixedDir(remoteRootLogDir, user, suffix), appId.toString());
>         }
>     
>         // Borrowed code from org.apache.hadoop.yarn.logaggregation.LogCLIHelpers
>         public int dumpAllContainersLogs(ApplicationId appId, String appOwner, PrintStream out) throws IOException {
>             Path remoteRootLogDir = new Path(hadoopConfig.get(YarnConfiguration.NM_REMOTE_APP_LOG_DIR,
>                     YarnConfiguration.DEFAULT_NM_REMOTE_APP_LOG_DIR));
>             String logDirSuffix = getRemoteNodeLogDirSuffix(hadoopConfig);
>             Path remoteAppLogDir = getRemoteAppLogDir(remoteRootLogDir, appId, appOwner, logDirSuffix);
>     
>             RemoteIterator nodeFiles;
>             try {
>                 Path qualifiedLogDir = FileContext.getFileContext(hadoopConfig).makeQualified(remoteAppLogDir);
>                 nodeFiles = FileContext.getFileContext(qualifiedLogDir.toUri(), hadoopConfig).listStatus(remoteAppLogDir);
>             } catch (FileNotFoundException fileNotFoundException) {
>                 logDirNotExist(remoteAppLogDir.toString());
>                 return -1;
>             }
>     
>             boolean foundAnyLogs = false;
>     
>             while(true) {
>                 FileStatus thisNodeFile;
>                 do {
>                     if (!nodeFiles.hasNext()) {
>                         if (!foundAnyLogs) {
>                             emptyLogDir(remoteAppLogDir.toString());
>                             return -1;
>                         }
>     
>                         return 0;
>                     }
>     
>                     thisNodeFile = (FileStatus)nodeFiles.next();
>                 } while(thisNodeFile.getPath().getName().endsWith(TMP_FILE_SUFFIX));
>     
>                 AggregatedLogFormat.LogReader reader = new AggregatedLogFormat.LogReader(hadoopConfig, thisNodeFile.getPath());
>     
>                 try {
>                     AggregatedLogFormat.LogKey key = new AggregatedLogFormat.LogKey();
>                     DataInputStream valueStream = reader.next(key);
>     
>                     while(valueStream != null) {
>                         String containerString = "\n\nContainer: " + key + " on " + thisNodeFile.getPath().getName();
>                         out.println(containerString);
>                         out.println(StringUtils.repeat("=", containerString.length()));
>     
>                         while(true) {
>                             try {
>                                 AggregatedLogFormat.LogReader.readAContainerLogsForALogType(valueStream, out,
>                                         thisNodeFile.getModificationTime());
>                                 foundAnyLogs = true;
>                             } catch (EOFException eofException) {
>                                 key = new AggregatedLogFormat.LogKey();
>                                 valueStream = reader.next(key);
>                                 break;
>                             }
>                         }
>                     }
>                 } finally {
>                     reader.close();
>                 }
>             }
>         }
>     }
>     
>     ```

I think this is an acceptable solution until we figure OOZIE-2983 out. We might try to push for a public API for this in Hadoop itself so we can just use that.


- Peter


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


On Oct. 4, 2017, 2:18 p.m., Attila Sasvari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62459/
> -----------------------------------------------------------
> 
> (Updated Oct. 4, 2017, 2:18 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 0b94484da1c97618e9168cea0ebbfff7f70f723c 
>   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/DiagBundleCollectorDriver.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleCompressor.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleEntryWriter.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagOozieClient.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/10/
> 
> 
> 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
> 
>


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

Posted by Attila Sasvari via Review Board <no...@reviews.apache.org>.

> On Oct. 6, 2017, 12:26 a.m., Robert Kanter wrote:
> > tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
> > Lines 173-174 (patched)
> > <https://reviews.apache.org/r/62459/diff/10/?file=1845962#file1845962line173>
> >
> >     ````LogAggregationUtils```` is marked ````@Private```` so we shouldn't be using it.  Hadoop can change things incompatibly here.

What do you think about borrowing/inlining/copying those functions from Hadoop 2.6 too? I was thinking about it and it is probably better than bringing back the initial ``ExecutorService`` based approach. I am thinking of a class like:

```java
// TODO: once OOZIE-2983 ("Stream the Launcher AM Logs") is done, remove it.
public class OozieLauncherLogFetcher {
    private static final String TMP_FILE_SUFFIX = ".tmp";
    final private Configuration hadoopConfig;

    public OozieLauncherLogFetcher(final Configuration hadoopConfig) {
        this.hadoopConfig = hadoopConfig;
    }

    // Borrowed code from org.apache.hadoop.yarn.logaggregation.LogCLIHelpers
    private static void logDirNotExist(String remoteAppLogDir) {
        System.out.println(remoteAppLogDir + "does not exist.");
        System.out.println("Log aggregation has not completed or is not enabled.");
    }
    // Borrowed code from org.apache.hadoop.yarn.logaggregation.LogCLIHelpers
    private static void emptyLogDir(String remoteAppLogDir) {
        System.out.println(remoteAppLogDir + "does not have any log files.");
    }
    // Borrowed code from org.apache.hadoop.yarn.logaggregation.LogAggregationUtils
    public static String getRemoteNodeLogDirSuffix(Configuration conf) {
        return conf.get(YarnConfiguration.NM_REMOTE_APP_LOG_DIR_SUFFIX, YarnConfiguration.DEFAULT_NM_REMOTE_APP_LOG_DIR_SUFFIX);
    }
    // Borrowed code from org.apache.hadoop.yarn.logaggregation.LogAggregationUtils
    public static Path getRemoteLogSuffixedDir(Path remoteRootLogDir, String user, String suffix) {
        return suffix != null && !suffix.isEmpty() ? new Path(getRemoteLogUserDir(remoteRootLogDir, user), suffix) :
                getRemoteLogUserDir(remoteRootLogDir, user);
    }
    // Borrowed code from org.apache.hadoop.yarn.logaggregation.LogAggregationUtils
    public static Path getRemoteLogUserDir(Path remoteRootLogDir, String user) {
        return new Path(remoteRootLogDir, user);
    }

    // Borrowed code from org.apache.hadoop.yarn.logaggregation.LogAggregationUtils
    public static Path getRemoteAppLogDir(Path remoteRootLogDir, ApplicationId appId, String user, String suffix) {
        return new Path(getRemoteLogSuffixedDir(remoteRootLogDir, user, suffix), appId.toString());
    }

    // Borrowed code from org.apache.hadoop.yarn.logaggregation.LogCLIHelpers
    public int dumpAllContainersLogs(ApplicationId appId, String appOwner, PrintStream out) throws IOException {
        Path remoteRootLogDir = new Path(hadoopConfig.get(YarnConfiguration.NM_REMOTE_APP_LOG_DIR,
                YarnConfiguration.DEFAULT_NM_REMOTE_APP_LOG_DIR));
        String logDirSuffix = getRemoteNodeLogDirSuffix(hadoopConfig);
        Path remoteAppLogDir = getRemoteAppLogDir(remoteRootLogDir, appId, appOwner, logDirSuffix);

        RemoteIterator nodeFiles;
        try {
            Path qualifiedLogDir = FileContext.getFileContext(hadoopConfig).makeQualified(remoteAppLogDir);
            nodeFiles = FileContext.getFileContext(qualifiedLogDir.toUri(), hadoopConfig).listStatus(remoteAppLogDir);
        } catch (FileNotFoundException fileNotFoundException) {
            logDirNotExist(remoteAppLogDir.toString());
            return -1;
        }

        boolean foundAnyLogs = false;

        while(true) {
            FileStatus thisNodeFile;
            do {
                if (!nodeFiles.hasNext()) {
                    if (!foundAnyLogs) {
                        emptyLogDir(remoteAppLogDir.toString());
                        return -1;
                    }

                    return 0;
                }

                thisNodeFile = (FileStatus)nodeFiles.next();
            } while(thisNodeFile.getPath().getName().endsWith(TMP_FILE_SUFFIX));

            AggregatedLogFormat.LogReader reader = new AggregatedLogFormat.LogReader(hadoopConfig, thisNodeFile.getPath());

            try {
                AggregatedLogFormat.LogKey key = new AggregatedLogFormat.LogKey();
                DataInputStream valueStream = reader.next(key);

                while(valueStream != null) {
                    String containerString = "\n\nContainer: " + key + " on " + thisNodeFile.getPath().getName();
                    out.println(containerString);
                    out.println(StringUtils.repeat("=", containerString.length()));

                    while(true) {
                        try {
                            AggregatedLogFormat.LogReader.readAContainerLogsForALogType(valueStream, out,
                                    thisNodeFile.getModificationTime());
                            foundAnyLogs = true;
                        } catch (EOFException eofException) {
                            key = new AggregatedLogFormat.LogKey();
                            valueStream = reader.next(key);
                            break;
                        }
                    }
                }
            } finally {
                reader.close();
            }
        }
    }
}

```


- Attila


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


On Oct. 4, 2017, 2:18 p.m., Attila Sasvari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62459/
> -----------------------------------------------------------
> 
> (Updated Oct. 4, 2017, 2:18 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 0b94484da1c97618e9168cea0ebbfff7f70f723c 
>   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/DiagBundleCollectorDriver.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleCompressor.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleEntryWriter.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagOozieClient.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/10/
> 
> 
> 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
> 
>


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

Posted by Robert Kanter <rk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62459/#review187223
-----------------------------------------------------------




docs/src/site/twiki/DG_CommandLineTool.twiki
Lines 1989 (patched)
<https://reviews.apache.org/r/62459/#comment264151>

    Shouldn't this "Done" show up on the previous line like the rest of them?



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 173-174 (patched)
<https://reviews.apache.org/r/62459/#comment264153>

    ````LogAggregationUtils```` is marked ````@Private```` so we shouldn't be using it.  Hadoop can change things incompatibly here.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 180 (patched)
<https://reviews.apache.org/r/62459/#comment264152>

    ````var20```` is a funny name for a variable :)



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 241 (patched)
<https://reviews.apache.org/r/62459/#comment264155>

    ````storeOozieLauncherLog```` should be called per Action, not per Job.  Launcher logs are an Action-level concept.
    
    Here, you're just picking the first action's external id to use for the job, which may not even exist (e.g. the first action is a decision node or an hdfs node.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 242 (patched)
<https://reviews.apache.org/r/62459/#comment264154>

    We should get this from the ````WorkflowJob````.  The log reading code computes a path based on the user who ran the job, which is likely to be different from the user running this tool.


- Robert Kanter


On Oct. 4, 2017, 2:18 p.m., Attila Sasvari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62459/
> -----------------------------------------------------------
> 
> (Updated Oct. 4, 2017, 2:18 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 0b94484da1c97618e9168cea0ebbfff7f70f723c 
>   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/DiagBundleCollectorDriver.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleCompressor.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleEntryWriter.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagOozieClient.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/10/
> 
> 
> 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
> 
>


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

Posted by András Piros via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62459/#review191045
-----------------------------------------------------------


Ship it!




Ship It!

- András Piros


On Oct. 16, 2017, 12:32 p.m., Attila Sasvari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62459/
> -----------------------------------------------------------
> 
> (Updated Oct. 16, 2017, 12:32 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 be153c16c45f91a8d57c78d6b7ebe073e7ca1f79 
>   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/DiagBundleCollectorDriver.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleCompressor.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleEntryWriter.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagOozieClient.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/OozieLauncherLogFetcher.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/12/
> 
> 
> 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
> 
>


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

Posted by Attila Sasvari via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62459/
-----------------------------------------------------------

(Updated Oct. 16, 2017, 12:32 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 (updated)
-----

  docs/src/site/twiki/DG_CommandLineTool.twiki d4047671876dcc3279a2ec379bc1d003f5e6f1aa 
  pom.xml be153c16c45f91a8d57c78d6b7ebe073e7ca1f79 
  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/DiagBundleCollectorDriver.java PRE-CREATION 
  tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleCompressor.java PRE-CREATION 
  tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleEntryWriter.java PRE-CREATION 
  tools/src/main/java/org/apache/oozie/tools/diag/DiagOozieClient.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/OozieLauncherLogFetcher.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/12/

Changes: https://reviews.apache.org/r/62459/diff/11-12/


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


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

Posted by Attila Sasvari via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62459/
-----------------------------------------------------------

(Updated Oct. 16, 2017, 7:11 a.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 (updated)
-----

  docs/src/site/twiki/DG_CommandLineTool.twiki d4047671876dcc3279a2ec379bc1d003f5e6f1aa 
  pom.xml be153c16c45f91a8d57c78d6b7ebe073e7ca1f79 
  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/DiagBundleCollectorDriver.java PRE-CREATION 
  tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleCompressor.java PRE-CREATION 
  tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleEntryWriter.java PRE-CREATION 
  tools/src/main/java/org/apache/oozie/tools/diag/DiagOozieClient.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/OozieLauncherLogFetcher.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/11/

Changes: https://reviews.apache.org/r/62459/diff/10-11/


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


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

Posted by Attila Sasvari <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62459/
-----------------------------------------------------------

(Updated Oct. 4, 2017, 2:18 p.m.)


Review request for oozie.


Changes
-------

New approach to fetch oozie launcher logs.


Repository: oozie-git


Description
-------

A diagnostic tool that collects a bunch of job and other information from Oozie in a zip file.


Diffs (updated)
-----

  docs/src/site/twiki/DG_CommandLineTool.twiki d4047671876dcc3279a2ec379bc1d003f5e6f1aa 
  pom.xml 0b94484da1c97618e9168cea0ebbfff7f70f723c 
  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/DiagBundleCollectorDriver.java PRE-CREATION 
  tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleCompressor.java PRE-CREATION 
  tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleEntryWriter.java PRE-CREATION 
  tools/src/main/java/org/apache/oozie/tools/diag/DiagOozieClient.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/10/

Changes: https://reviews.apache.org/r/62459/diff/9-10/


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


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

Posted by Attila Sasvari <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62459/
-----------------------------------------------------------

(Updated Oct. 2, 2017, 9:39 a.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 (updated)
-----

  docs/src/site/twiki/DG_CommandLineTool.twiki d4047671876dcc3279a2ec379bc1d003f5e6f1aa 
  pom.xml 0b94484da1c97618e9168cea0ebbfff7f70f723c 
  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/DiagBundleCollectorDriver.java PRE-CREATION 
  tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleCompressor.java PRE-CREATION 
  tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleEntryWriter.java PRE-CREATION 
  tools/src/main/java/org/apache/oozie/tools/diag/DiagOozieClient.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/8/

Changes: https://reviews.apache.org/r/62459/diff/7-8/


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


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

Posted by Attila Sasvari <as...@cloudera.com>.

> On Sept. 30, 2017, 4:41 a.m., Robert Kanter 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>
> >
> >     Something to consider for all output of the tool, not just here: we're outputting most of the info in a human-readable format.  Should we think about using a machine-readable format?  Or maybe having the option for one?  Or doing both?  The idea being that someone would then be able to write their own tool that could analyze stuff.  We already have some code somewhere that converts a WorkflowJob into JSON, so it shouldn't be a lot of work to add this either.  That might also be a good idea from a compatibility perspective - i.e. what's the compatibility story on this out?  If there's a new field, what do we do?

It is a good idea, and I would create a separate JIRA to discuss/design and implement it. OOZIE-3074


> On Sept. 30, 2017, 4:41 a.m., Robert Kanter wrote:
> > tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
> > Lines 163 (patched)
> > <https://reviews.apache.org/r/62459/diff/7/?file=1837523#file1837523line163>
> >
> >     I think the JHS may also be required, in the cases where the RM has forgotten about the job.
> >     
> >     And what about HDFS?  That's required too.
> >     
> >     I'm thinking we might be best off not doing these checks.  It's too complicated (CM spent a lot of effort on this) and we can't check for everything (e.g. what if log aggregation is turned off?).  Besides, we're already handling exceptions below when trying to get the logs - if the RM, JHS, HDFS, etc isn't working, the call will fail anyway.

I agree with it, but failing fast would be a better experience than a 30 seconds timeout. The default retry policy of YarnClient resulted in a lot of retries and I could not find the proper parameter to control it (I want to restrict retries to a few seconds instead of minutes).


> On Sept. 30, 2017, 4:41 a.m., Robert Kanter wrote:
> > tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
> > Lines 185 (patched)
> > <https://reviews.apache.org/r/62459/diff/7/?file=1837523#file1837523line185>
> >
> >     Please create a followup JIRA to change this in the future to use OOZIE-2983 ("Stream the Launcher AM Logs") once it's done.  This will also be nice in that we can get rid of the RM up check.

I will do so.


> On Sept. 30, 2017, 4:41 a.m., Robert Kanter wrote:
> > tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
> > Lines 191 (patched)
> > <https://reviews.apache.org/r/62459/diff/7/?file=1837523#file1837523line191>
> >
> >     Is there not a cleaner way to do this than using a CLI like this?

I could not find one, please let me know if you have something in mind.


> On Sept. 30, 2017, 4:41 a.m., Robert Kanter wrote:
> > tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
> > Lines 221 (patched)
> > <https://reviews.apache.org/r/62459/diff/7/?file=1837523#file1837523line221>
> >
> >     This won't work right if using RM HA...
> >     
> >     I'd recommend using a ````YarnClient```` and passing it the ````hadoopConfig```` so it can figure out the RM address for you.  There must be a benign simple ````YarnClient```` command you can run to verify connectivity.

I could not find such command, but please let me know which one do you think of. Methods I tried retried to connect to RM multiple times for more minutes in case of a connection error.


> On Sept. 30, 2017, 4:41 a.m., Robert Kanter wrote:
> > tools/src/main/java/org/apache/oozie/tools/diag/BundleCollectorDriver.java
> > Lines 37 (patched)
> > <https://reviews.apache.org/r/62459/diff/7/?file=1837525#file1837525line37>
> >
> >     I'm not sure I like the name "BundleXYZ" for these classes.  It's ambiguous with a Bundle Job.  Perhaps 
> >     "DiagBundleXYZ" instead?

These classes are in the ``org.apache.oozie.tools.diag`` package that why I thought names like Client, BundleXYZ are not ambigous. Will fix it.


- Attila


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


On Oct. 2, 2017, 9:39 a.m., Attila Sasvari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62459/
> -----------------------------------------------------------
> 
> (Updated Oct. 2, 2017, 9:39 a.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 0b94484da1c97618e9168cea0ebbfff7f70f723c 
>   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/DiagBundleCollectorDriver.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleCompressor.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleEntryWriter.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagOozieClient.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/8/
> 
> 
> 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
> 
>


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

Posted by Attila Sasvari via Review Board <no...@reviews.apache.org>.

> On Sept. 30, 2017, 4:41 a.m., Robert Kanter wrote:
> > tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
> > Lines 151 (patched)
> > <https://reviews.apache.org/r/62459/diff/7/?file=1837523#file1837523line151>
> >
> >     Should we skip control types?  That's probably fine for the start action, but what about a decision action?  That has some useful info.  Maybe we should still print these out, but with fewer fields or something (e.g. no need for a console url).
> 
> Robert Kanter wrote:
>     Thinking about this more, I think we should include control types.  It will be helpful for following the flow of a workflow when there's decision and fork nodes.  Please update the patch to handle this.  We don't need to print out all of the fields, so you'll have to figure out which ones make sense.
> 
> Attila Sasvari wrote:
>     Sure, I will do so.

I believe only the decision node might interesting to see execution path, but I am not sure. ``info.txt`` already contains execution / transition info:
```
Action Id          : "0000004-171015215418832-oozie-asas-W@ctrl"
Name               : "ctrl"
Type               : "switch"
Status             : "OK"
Transition         : "forking"
Start Time         : "2017-10-15 20:20:36 GMT"
End Time           : "2017-10-15 20:20:36 GMT"
Error Code         : null
Error Message      : null
Console URL        : "-"
Tracker URI        : "-"
External Child Ids : null
External Id        : "-"
External Status    : "forking"
Data               : null
Stats              : null
Credentials        : null
Retries            : "0"
User Retry Int     : "10"
User Retry Count   : "0"
User Retry Max     : "0"
```

If I store resolved action defintion, it would not add too much information:
```
# cat 0000004-171015215418832-oozie-asas-W/resolved-actions/ctrl.xml      
<switch xmlns="uri:oozie:workflow:0.2">
  <case to="forking">true</case>
  <default to="end" />
</switch>% 
```

I created a workflow with a fork node and also with a decision node. I saw that resolved action config xml were empty for control nodes other than the decision node.

```
        0  10-15-17 22:44   /0000004-171015215418832-oozie-asas-W/resolved-actions/:start:.xml
      108  10-15-17 22:44   /0000004-171015215418832-oozie-asas-W/resolved-actions/ctrl.xml
        0  10-15-17 22:44   /0000004-171015215418832-oozie-asas-W/resolved-actions/end.xml
        0  10-15-17 22:44   /0000004-171015215418832-oozie-asas-W/resolved-actions/forking.xml
        0  10-15-17 22:44   /0000004-171015215418832-oozie-asas-W/resolved-actions/join.xml
    83834  10-15-17 22:44   /0000004-171015215418832-oozie-asas-W/resolved-actions/launcher_mr-node.log
    83843  10-15-17 22:44   /0000004-171015215418832-oozie-asas-W/resolved-actions/launcher_mr-node2.log
    47965  10-15-17 22:44   /0000004-171015215418832-oozie-asas-W/resolved-actions/launcher_shell1.log
    47965  10-15-17 22:44   /0000004-171015215418832-oozie-asas-W/resolved-actions/launcher_shell2.log
     2045  10-15-17 22:44   /0000004-171015215418832-oozie-asas-W/resolved-actions/mr-node.xml
     2045  10-15-17 22:44   /0000004-171015215418832-oozie-asas-W/resolved-actions/mr-node2.xml
      544  10-15-17 22:44   /0000004-171015215418832-oozie-asas-W/resolved-actions/shell1.xml
      544  10-15-17 22:44   /0000004-171015215418832-oozie-asas-W/resolved-actions/shell2.xml
```
Zero byte xml-s are not interesting. I will remove non-decision action types.

Other thing: it would also good to add later an option to download the final job graph (thanks to OOZIE-2406 now we can download it as a Dot file too). If they have many action, it might help a lot when taking a quick look. 

Misc: Would it better to have a different pattern for the filename? For example: it might be good to see where the bundle was actually taken (i.e. include fqdn-hostname, there are existing diagnostic tools that use similar pattern).


- Attila


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


On Oct. 4, 2017, 2:18 p.m., Attila Sasvari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62459/
> -----------------------------------------------------------
> 
> (Updated Oct. 4, 2017, 2:18 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 0b94484da1c97618e9168cea0ebbfff7f70f723c 
>   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/DiagBundleCollectorDriver.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleCompressor.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleEntryWriter.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagOozieClient.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/10/
> 
> 
> 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
> 
>


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

Posted by Attila Sasvari <as...@cloudera.com>.

> On Sept. 30, 2017, 4:41 a.m., Robert Kanter wrote:
> > tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
> > Lines 191 (patched)
> > <https://reviews.apache.org/r/62459/diff/7/?file=1837523#file1837523line191>
> >
> >     Is there not a cleaner way to do this than using a CLI like this?
> 
> Attila Sasvari wrote:
>     I could not find one, please let me know if you have something in mind.

OK, I am going to inline/borrow code from dumpAllContainersLogs in LogsCLI. What we do is basically just fetching logs from HDFS. If there is an exception, we log it and move on.


- Attila


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


On Oct. 2, 2017, 9:39 a.m., Attila Sasvari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62459/
> -----------------------------------------------------------
> 
> (Updated Oct. 2, 2017, 9:39 a.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 0b94484da1c97618e9168cea0ebbfff7f70f723c 
>   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/DiagBundleCollectorDriver.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleCompressor.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleEntryWriter.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagOozieClient.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/9/
> 
> 
> 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
> 
>


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

Posted by Robert Kanter <rk...@cloudera.com>.

> On Sept. 30, 2017, 4:41 a.m., Robert Kanter wrote:
> > tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
> > Lines 151 (patched)
> > <https://reviews.apache.org/r/62459/diff/7/?file=1837523#file1837523line151>
> >
> >     Should we skip control types?  That's probably fine for the start action, but what about a decision action?  That has some useful info.  Maybe we should still print these out, but with fewer fields or something (e.g. no need for a console url).

Thinking about this more, I think we should include control types.  It will be helpful for following the flow of a workflow when there's decision and fork nodes.  Please update the patch to handle this.  We don't need to print out all of the fields, so you'll have to figure out which ones make sense.


- Robert


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


On Oct. 4, 2017, 2:18 p.m., Attila Sasvari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62459/
> -----------------------------------------------------------
> 
> (Updated Oct. 4, 2017, 2:18 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 0b94484da1c97618e9168cea0ebbfff7f70f723c 
>   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/DiagBundleCollectorDriver.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleCompressor.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleEntryWriter.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagOozieClient.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/10/
> 
> 
> 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
> 
>


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

Posted by Robert Kanter <rk...@cloudera.com>.

> On Sept. 30, 2017, 4:41 a.m., Robert Kanter wrote:
> > tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
> > Lines 221 (patched)
> > <https://reviews.apache.org/r/62459/diff/7/?file=1837523#file1837523line221>
> >
> >     This won't work right if using RM HA...
> >     
> >     I'd recommend using a ````YarnClient```` and passing it the ````hadoopConfig```` so it can figure out the RM address for you.  There must be a benign simple ````YarnClient```` command you can run to verify connectivity.
> 
> Attila Sasvari wrote:
>     I could not find such command, but please let me know which one do you think of. Methods I tried retried to connect to RM multiple times for more minutes in case of a connection error.

Ya, apparently there isn't really one.  Hadoop 3 has a CLI command that would tell you the state of each RM (https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HAAdmin.java#L481)  Maybe you can steal something from it?  I'm not really sure of a good solution here.


- Robert


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


On Oct. 2, 2017, 9:39 a.m., Attila Sasvari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62459/
> -----------------------------------------------------------
> 
> (Updated Oct. 2, 2017, 9:39 a.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 0b94484da1c97618e9168cea0ebbfff7f70f723c 
>   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/DiagBundleCollectorDriver.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleCompressor.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleEntryWriter.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagOozieClient.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/9/
> 
> 
> 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
> 
>


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

Posted by Attila Sasvari via Review Board <no...@reviews.apache.org>.

> On Sept. 30, 2017, 4:41 a.m., Robert Kanter wrote:
> > tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
> > Lines 151 (patched)
> > <https://reviews.apache.org/r/62459/diff/7/?file=1837523#file1837523line151>
> >
> >     Should we skip control types?  That's probably fine for the start action, but what about a decision action?  That has some useful info.  Maybe we should still print these out, but with fewer fields or something (e.g. no need for a console url).
> 
> Robert Kanter wrote:
>     Thinking about this more, I think we should include control types.  It will be helpful for following the flow of a workflow when there's decision and fork nodes.  Please update the patch to handle this.  We don't need to print out all of the fields, so you'll have to figure out which ones make sense.
> 
> Attila Sasvari wrote:
>     Sure, I will do so.
> 
> Attila Sasvari wrote:
>     I believe only the decision node might interesting to see execution path, but I am not sure. ``info.txt`` already contains execution / transition info:
>     ```
>     Action Id          : "0000004-171015215418832-oozie-asas-W@ctrl"
>     Name               : "ctrl"
>     Type               : "switch"
>     Status             : "OK"
>     Transition         : "forking"
>     Start Time         : "2017-10-15 20:20:36 GMT"
>     End Time           : "2017-10-15 20:20:36 GMT"
>     Error Code         : null
>     Error Message      : null
>     Console URL        : "-"
>     Tracker URI        : "-"
>     External Child Ids : null
>     External Id        : "-"
>     External Status    : "forking"
>     Data               : null
>     Stats              : null
>     Credentials        : null
>     Retries            : "0"
>     User Retry Int     : "10"
>     User Retry Count   : "0"
>     User Retry Max     : "0"
>     ```
>     
>     If I store resolved action defintion, it would not add too much information:
>     ```
>     # cat 0000004-171015215418832-oozie-asas-W/resolved-actions/ctrl.xml      
>     <switch xmlns="uri:oozie:workflow:0.2">
>       <case to="forking">true</case>
>       <default to="end" />
>     </switch>% 
>     ```
>     
>     I created a workflow with a fork node and also with a decision node. I saw that resolved action config xml were empty for control nodes other than the decision node.
>     
>     ```
>             0  10-15-17 22:44   /0000004-171015215418832-oozie-asas-W/resolved-actions/:start:.xml
>           108  10-15-17 22:44   /0000004-171015215418832-oozie-asas-W/resolved-actions/ctrl.xml
>             0  10-15-17 22:44   /0000004-171015215418832-oozie-asas-W/resolved-actions/end.xml
>             0  10-15-17 22:44   /0000004-171015215418832-oozie-asas-W/resolved-actions/forking.xml
>             0  10-15-17 22:44   /0000004-171015215418832-oozie-asas-W/resolved-actions/join.xml
>         83834  10-15-17 22:44   /0000004-171015215418832-oozie-asas-W/resolved-actions/launcher_mr-node.log
>         83843  10-15-17 22:44   /0000004-171015215418832-oozie-asas-W/resolved-actions/launcher_mr-node2.log
>         47965  10-15-17 22:44   /0000004-171015215418832-oozie-asas-W/resolved-actions/launcher_shell1.log
>         47965  10-15-17 22:44   /0000004-171015215418832-oozie-asas-W/resolved-actions/launcher_shell2.log
>          2045  10-15-17 22:44   /0000004-171015215418832-oozie-asas-W/resolved-actions/mr-node.xml
>          2045  10-15-17 22:44   /0000004-171015215418832-oozie-asas-W/resolved-actions/mr-node2.xml
>           544  10-15-17 22:44   /0000004-171015215418832-oozie-asas-W/resolved-actions/shell1.xml
>           544  10-15-17 22:44   /0000004-171015215418832-oozie-asas-W/resolved-actions/shell2.xml
>     ```
>     Zero byte xml-s are not interesting. I will remove non-decision action types.
>     
>     Other thing: it would also good to add later an option to download the final job graph (thanks to OOZIE-2406 now we can download it as a Dot file too). If they have many action, it might help a lot when taking a quick look. 
>     
>     Misc: Would it better to have a different pattern for the filename? For example: it might be good to see where the bundle was actually taken (i.e. include fqdn-hostname, there are existing diagnostic tools that use similar pattern).

update: I talked with Andras Piros and gezapeti and decided to include control nodes. It makes it easy to see whether the workflow failed or not (you don't need to open the info.txt).

 
```
Archive:  /Users/asasvari/workspace/apache/oozie_dup/distro/target/oozie-5.0.0-SNAPSHOT-distro/oozie-5.0.0-SNAPSHOT/./oozie-diag-bundle-1508156773346.zip
  Length     Date   Time    Name
 --------    ----   ----    ----
     7843  10-16-17 14:26   /0000006-171015215418832-oozie-asas-W/info.txt
      322  10-16-17 14:26   /0000006-171015215418832-oozie-asas-W/job.properties
    20878  10-16-17 14:26   /0000006-171015215418832-oozie-asas-W/log.txt
        0  10-16-17 14:26   /0000006-171015215418832-oozie-asas-W/resolved-actions/
        0  10-16-17 14:26   /0000006-171015215418832-oozie-asas-W/resolved-actions/:start:.xml
      108  10-16-17 14:26   /0000006-171015215418832-oozie-asas-W/resolved-actions/ctrl.xml
        0  10-16-17 14:26   /0000006-171015215418832-oozie-asas-W/resolved-actions/end.xml
        0  10-16-17 14:26   /0000006-171015215418832-oozie-asas-W/resolved-actions/forking.xml
        0  10-16-17 14:26   /0000006-171015215418832-oozie-asas-W/resolved-actions/join.xml
    83827  10-16-17 14:26   /0000006-171015215418832-oozie-asas-W/resolved-actions/launcher_mr-node.log
    83836  10-16-17 14:26   /0000006-171015215418832-oozie-asas-W/resolved-actions/launcher_mr-node2.log
    47956  10-16-17 14:26   /0000006-171015215418832-oozie-asas-W/resolved-actions/launcher_shell1.log
    47956  10-16-17 14:26   /0000006-171015215418832-oozie-asas-W/resolved-actions/launcher_shell2.log
     2045  10-16-17 14:26   /0000006-171015215418832-oozie-asas-W/resolved-actions/mr-node.xml
     2045  10-16-17 14:26   /0000006-171015215418832-oozie-asas-W/resolved-actions/mr-node2.xml
      544  10-16-17 14:26   /0000006-171015215418832-oozie-asas-W/resolved-actions/shell1.xml
      543  10-16-17 14:26   /0000006-171015215418832-oozie-asas-W/resolved-actions/shell2.xml
     5495  10-16-17 14:26   /0000006-171015215418832-oozie-asas-W/workflow.xml
     7268  10-16-17 14:26   /0000007-171015215418832-oozie-asas-W/info.txt
      322  10-16-17 14:26   /0000007-171015215418832-oozie-asas-W/job.properties
    22429  10-16-17 14:26   /0000007-171015215418832-oozie-asas-W/log.txt
        0  10-16-17 14:26   /0000007-171015215418832-oozie-asas-W/resolved-actions/
        0  10-16-17 14:26   /0000007-171015215418832-oozie-asas-W/resolved-actions/:start:.xml
      108  10-16-17 14:26   /0000007-171015215418832-oozie-asas-W/resolved-actions/ctrl.xml
       72  10-16-17 14:26   /0000007-171015215418832-oozie-asas-W/resolved-actions/fail.xml
        0  10-16-17 14:26   /0000007-171015215418832-oozie-asas-W/resolved-actions/forking.xml
    83827  10-16-17 14:26   /0000007-171015215418832-oozie-asas-W/resolved-actions/launcher_mr-node.log
    83836  10-16-17 14:26   /0000007-171015215418832-oozie-asas-W/resolved-actions/launcher_mr-node2.log
    47957  10-16-17 14:26   /0000007-171015215418832-oozie-asas-W/resolved-actions/launcher_shell1.log
    51332  10-16-17 14:26   /0000007-171015215418832-oozie-asas-W/resolved-actions/launcher_shell2.log
     2045  10-16-17 14:26   /0000007-171015215418832-oozie-asas-W/resolved-actions/mr-node.xml
     2045  10-16-17 14:26   /0000007-171015215418832-oozie-asas-W/resolved-actions/mr-node2.xml
      544  10-16-17 14:26   /0000007-171015215418832-oozie-asas-W/resolved-actions/shell1.xml
      543  10-16-17 14:26   /0000007-171015215418832-oozie-asas-W/resolved-actions/shell2.xml
     5495  10-16-17 14:26   /0000007-171015215418832-oozie-asas-W/workflow.xml
    64567  10-16-17 14:26   /effective-oozie-site.xml
    32946  10-16-17 14:26   /instrumentation.txt
    32712  10-16-17 14:26   /java-sys-props.txt
     3643  10-16-17 14:26   /os-env-vars.txt
      279  10-16-17 14:26   /queue-dump.txt
    38906  10-16-17 14:26   /sharelib.txt
   103433  10-16-17 14:26   /thread-dump.html
```


- Attila


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


On Oct. 16, 2017, 7:11 a.m., Attila Sasvari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62459/
> -----------------------------------------------------------
> 
> (Updated Oct. 16, 2017, 7:11 a.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 be153c16c45f91a8d57c78d6b7ebe073e7ca1f79 
>   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/DiagBundleCollectorDriver.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleCompressor.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagBundleEntryWriter.java PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/diag/DiagOozieClient.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/OozieLauncherLogFetcher.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/11/
> 
> 
> 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
> 
>


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

Posted by Robert Kanter <rk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62459/#review186754
-----------------------------------------------------------




docs/src/site/twiki/DG_CommandLineTool.twiki
Lines 1948 (patched)
<https://reviews.apache.org/r/62459/#comment263555>

    It would be good to add something about the children aspect of the tool (e.g. that if you get a coordinator, it will also get some number of child workflows)



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 97-98 (patched)
<https://reviews.apache.org/r/62459/#comment263557>

    Something to consider for all output of the tool, not just here: we're outputting most of the info in a human-readable format.  Should we think about using a machine-readable format?  Or maybe having the option for one?  Or doing both?  The idea being that someone would then be able to write their own tool that could analyze stuff.  We already have some code somewhere that converts a WorkflowJob into JSON, so it shouldn't be a lot of work to add this either.  That might also be a good idea from a compatibility perspective - i.e. what's the compatibility story on this out?  If there's a new field, what do we do?



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 99-119 (patched)
<https://reviews.apache.org/r/62459/#comment263556>

    I'm not sure that's necessary here - we're not concatinating strings, we're writing to a Writer.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 151 (patched)
<https://reviews.apache.org/r/62459/#comment263558>

    Should we skip control types?  That's probably fine for the start action, but what about a decision action?  That has some useful info.  Maybe we should still print these out, but with fewer fields or something (e.g. no need for a console url).



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 163 (patched)
<https://reviews.apache.org/r/62459/#comment263604>

    I think the JHS may also be required, in the cases where the RM has forgotten about the job.
    
    And what about HDFS?  That's required too.
    
    I'm thinking we might be best off not doing these checks.  It's too complicated (CM spent a lot of effort on this) and we can't check for everything (e.g. what if log aggregation is turned off?).  Besides, we're already handling exceptions below when trying to get the logs - if the RM, JHS, HDFS, etc isn't working, the call will fail anyway.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 185 (patched)
<https://reviews.apache.org/r/62459/#comment263560>

    Please create a followup JIRA to change this in the future to use OOZIE-2983 ("Stream the Launcher AM Logs") once it's done.  This will also be nice in that we can get rid of the RM up check.



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 191 (patched)
<https://reviews.apache.org/r/62459/#comment263559>

    Is there not a cleaner way to do this than using a CLI like this?



tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 221 (patched)
<https://reviews.apache.org/r/62459/#comment263561>

    This won't work right if using RM HA...
    
    I'd recommend using a ````YarnClient```` and passing it the ````hadoopConfig```` so it can figure out the RM address for you.  There must be a benign simple ````YarnClient```` command you can run to verify connectivity.



tools/src/main/java/org/apache/oozie/tools/diag/BundleCollectorDriver.java
Lines 37 (patched)
<https://reviews.apache.org/r/62459/#comment263605>

    I'm not sure I like the name "BundleXYZ" for these classes.  It's ambiguous with a Bundle Job.  Perhaps 
    "DiagBundleXYZ" instead?



tools/src/main/java/org/apache/oozie/tools/diag/BundleCompressor.java
Lines 32 (patched)
<https://reviews.apache.org/r/62459/#comment263606>

    Can use the Java 7 try-with-resources.



tools/src/main/java/org/apache/oozie/tools/diag/Client.java
Lines 32 (patched)
<https://reviews.apache.org/r/62459/#comment263607>

    I think you have a typo here: "... so we get that for free"



tools/src/main/java/org/apache/oozie/tools/diag/Client.java
Lines 38 (patched)
<https://reviews.apache.org/r/62459/#comment263609>

    I would add a comment here explaining why we're doing this.  (i.e. that Oozie has a jsp page with a thread dump, and the Oozie client normally doesn't have a way of getting it, so we're doing that here; reusing all the fancy HTTP handling code in AuthOozieClient)



tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java
Lines 47-49 (patched)
<https://reviews.apache.org/r/62459/#comment263608>

    Strange lining here



tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java
Lines 45-47 (patched)
<https://reviews.apache.org/r/62459/#comment263610>

    Strange lining here



tools/src/test/java/org/apache/oozie/tools/diag/TestAppInfoCollector.java
Lines 48 (patched)
<https://reviews.apache.org/r/62459/#comment263611>

    I know the ````@Test```` indicates that this is a test, but it's still convention to name all tests starting with "test".  So, "testGetLastNWorkflows()".  Same for the others.


- Robert Kanter


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


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

Posted by Peter Cseh <ge...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62459/#review186563
-----------------------------------------------------------




tools/src/test/java/org/apache/oozie/tools/diag/TestAppInfoCollector.java
Lines 1 (patched)
<https://reviews.apache.org/r/62459/#comment263118>

    License



tools/src/test/java/org/apache/oozie/tools/diag/TestServerInfoCollector.java
Lines 1 (patched)
<https://reviews.apache.org/r/62459/#comment263119>

    License


- Peter Cseh


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


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

Posted by Peter Cseh <ge...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62459/#review186561
-----------------------------------------------------------




tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java
Lines 164 (patched)
<https://reviews.apache.org/r/62459/#comment263115>

    Please log here that Launcher logs will be missing for this reason.


- Peter Cseh


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


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

Posted by Attila Sasvari <as...@cloudera.com>.
-----------------------------------------------------------
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.


Changes
-------

Added more tests for AppInfoCollector and others.


Repository: oozie-git


Description
-------

A diagnostic tool that collects a bunch of job and other information from Oozie in a zip file.


Diffs (updated)
-----

  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/

Changes: https://reviews.apache.org/r/62459/diff/6-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


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

Posted by Attila Sasvari <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62459/
-----------------------------------------------------------

(Updated Sept. 26, 2017, 9:03 p.m.)


Review request for oozie.


Changes
-------

more tests


Repository: oozie-git


Description
-------

A diagnostic tool that collects a bunch of job and other information from Oozie in a zip file.


Diffs (updated)
-----

  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/AppInfoCollectorTest.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/6/

Changes: https://reviews.apache.org/r/62459/diff/5-6/


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


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

Posted by Attila Sasvari <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62459/
-----------------------------------------------------------

(Updated Sept. 26, 2017, 10:51 a.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 (updated)
-----

  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/Client.java PRE-CREATION 
  tools/src/main/java/org/apache/oozie/tools/diag/ConfigEntryWriter.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/TestArgParser.java PRE-CREATION 


Diff: https://reviews.apache.org/r/62459/diff/5/

Changes: https://reviews.apache.org/r/62459/diff/4-5/


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


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

Posted by Attila Sasvari <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62459/
-----------------------------------------------------------

(Updated Sept. 25, 2017, 8:27 a.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 (updated)
-----

  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/OozieDiagBundleCollector.java PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/TestOozieDiagBundleCollector.java PRE-CREATION 


Diff: https://reviews.apache.org/r/62459/diff/4/

Changes: https://reviews.apache.org/r/62459/diff/3-4/


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


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

Posted by Attila Sasvari <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62459/
-----------------------------------------------------------

(Updated Sept. 22, 2017, 12:50 p.m.)


Review request for oozie.


Changes
-------

Collect Oozie launcher logs


Repository: oozie-git


Description
-------

A diagnostic tool that collects a bunch of job and other information from Oozie in a zip file.


Diffs (updated)
-----

  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/OozieDiagBundleCollector.java PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/TestOozieDiagBundleCollector.java PRE-CREATION 


Diff: https://reviews.apache.org/r/62459/diff/3/

Changes: https://reviews.apache.org/r/62459/diff/2-3/


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


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

Posted by Attila Sasvari <as...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62459/
-----------------------------------------------------------

(Updated Sept. 21, 2017, 5:10 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 (updated)
-----

  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/OozieDiagBundleCollector.java PRE-CREATION 
  tools/src/test/java/org/apache/oozie/tools/TestOozieDiagBundleCollector.java PRE-CREATION 


Diff: https://reviews.apache.org/r/62459/diff/2/

Changes: https://reviews.apache.org/r/62459/diff/1-2/


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


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

Posted by Peter Cseh <ge...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62459/#review185887
-----------------------------------------------------------




tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 869 (patched)
<https://reviews.apache.org/r/62459/#comment262226>

    I think we should distinguish "null" values from empty strings as this can be an important thing in configurations


- Peter Cseh


On Sept. 21, 2017, 10:31 a.m., Attila Sasvari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62459/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2017, 10:31 a.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 
>   tools/src/main/bin/oozie-diag-bundle-collector.sh PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java PRE-CREATION 
>   tools/src/test/java/org/apache/oozie/tools/TestOozieDiagBundleCollector.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62459/diff/1/
> 
> 
> 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
> 
>


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

Posted by Peter Cseh <ge...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62459/#review185884
-----------------------------------------------------------




tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java
Lines 141-148 (patched)
<https://reviews.apache.org/r/62459/#comment262223>

    Can these be fields so the argument lists are not that long?


- Peter Cseh


On Sept. 21, 2017, 10:31 a.m., Attila Sasvari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62459/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2017, 10:31 a.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 
>   tools/src/main/bin/oozie-diag-bundle-collector.sh PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java PRE-CREATION 
>   tools/src/test/java/org/apache/oozie/tools/TestOozieDiagBundleCollector.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62459/diff/1/
> 
> 
> 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
> 
>