Re: Review Request 21278: OOZIE-1817: Oozie timers are not biased

2014-06-16 Thread Mona Chitnis
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21278/#review45783 --- patch looks good. Please ensure that the cron.start() and

Re: Review Request 21278: OOZIE-1817: Oozie timers are not biased

2014-06-16 Thread Robert Kanter
On June 16, 2014, 6:35 p.m., Mona Chitnis wrote: core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java, line 115 https://reviews.apache.org/r/21278/diff/6/?file=588562#file588562line115 so no counter for inputcheck command? One of the parent

Re: Review Request 21278: OOZIE-1817: Oozie timers are not biased

2014-06-16 Thread Robert Kanter
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21278/ --- (Updated June 16, 2014, 8:09 p.m.) Review request for oozie. Changes ---

Re: Review Request 21278: OOZIE-1817: Oozie timers are not biased

2014-05-30 Thread Mona Chitnis
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21278/#review44419 --- some more comments in addition to the couple before

Re: Review Request 21278: OOZIE-1817: Oozie timers are not biased

2014-05-30 Thread Robert Kanter
On May 30, 2014, 9 p.m., Mona Chitnis wrote: core/src/main/java/org/apache/oozie/servlet/V2AdminServlet.java, line 53 https://reviews.apache.org/r/21278/diff/6/?file=588573#file588573line53 where is the disabling of instrumentation endpoint taking place? In V2AdminServlet, if

Re: Review Request 21278: OOZIE-1817: Oozie timers are not biased

2014-05-27 Thread Mona Chitnis
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21278/#review44075 --- Will look into Codahale a bit more in detail and review again.

Re: Review Request 21278: OOZIE-1817: Oozie timers are not biased

2014-05-22 Thread Robert Kanter
On May 21, 2014, 4:24 a.m., Gilad Wolff wrote: core/src/main/java/org/apache/oozie/util/MetricsInstrumentation.java, line 137 https://reviews.apache.org/r/21278/diff/4-5/?file=583904#file583904line137 I believe that Cache gives you the semantics you want and you don't need the

Re: Review Request 21278: OOZIE-1817: Oozie timers are not biased

2014-05-22 Thread Robert Kanter
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21278/ --- (Updated May 22, 2014, 11:27 p.m.) Review request for oozie. Changes ---

Re: Review Request 21278: OOZIE-1817: Oozie timers are not biased

2014-05-20 Thread Gilad Wolff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21278/#review43585 --- Ship it! nit-picks.

Re: Review Request 21278: OOZIE-1817: Oozie timers are not biased

2014-05-19 Thread Robert Kanter
On May 19, 2014, 4:10 a.m., Gilad Wolff wrote: core/src/main/java/org/apache/oozie/servlet/V2AdminServlet.java, line 53 https://reviews.apache.org/r/21278/diff/4/?file=583903#file583903line53 super nit: this seems like a very long line. We use max 132 characters per line (IMO, this

Re: Review Request 21278: OOZIE-1817: Oozie timers are not biased

2014-05-19 Thread Robert Kanter
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21278/ --- (Updated May 19, 2014, 6:46 p.m.) Review request for oozie. Changes ---

Re: Review Request 21278: OOZIE-1817: Oozie timers are not biased

2014-05-18 Thread Gilad Wolff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21278/#review4 --- Ship it! Thanks for the changes. A couple of comments and

Review Request 21278: OOZIE-1817: Oozie timers are not biased

2014-05-16 Thread Robert Kanter
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21278/ --- Review request for oozie. Bugs: OOZIE-1817

Re: Review Request 21278: OOZIE-1817: Oozie timers are not biased

2014-05-16 Thread Gilad Wolff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21278/#review42740 --- Ship it! Looks great, thanks for doing this. I have a few comments

Re: Review Request 21278: OOZIE-1817: Oozie timers are not biased

2014-05-16 Thread Robert Kanter
On May 15, 2014, 8:55 p.m., Gilad Wolff wrote: core/src/main/java/org/apache/oozie/util/MetricsInstrumentation.java, line 147 https://reviews.apache.org/r/21278/diff/3/?file=580658#file580658line147 = 0 ? what if something takes less than 1ms? Robert Kanter wrote: The

Re: Review Request 21278: OOZIE-1817: Oozie timers are not biased

2014-05-16 Thread Robert Kanter
On May 15, 2014, 8:55 p.m., Gilad Wolff wrote: core/src/main/java/org/apache/oozie/service/MetricsInstrumentationService.java, line 37 https://reviews.apache.org/r/21278/diff/3/?file=580650#file580650line37 nit: you don't really need this member. Use the base class member

Re: Review Request 21278: OOZIE-1817: Oozie timers are not biased

2014-05-13 Thread Robert Kanter
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21278/ --- (Updated May 13, 2014, 5:40 p.m.) Review request for oozie. Changes ---

Re: Review Request 21278: OOZIE-1817: Oozie timers are not biased

2014-05-12 Thread Robert Kanter
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21278/ --- (Updated May 12, 2014, 8:53 p.m.) Review request for oozie. Changes ---