Re: Review Request 33049: Patch for KAFKA-2084

2015-10-08 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/#review101976 --- core/src/main/scala/kafka/server/ClientQuotaManager.scala (line 14

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-18 Thread Aditya Auradkar
> On Aug. 18, 2015, 11:33 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/ClientQuotaManager.scala, line 147 > > > > > > The window used here is a bit different from that used in > > Rate.measure() and it

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-18 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/#review95793 --- clients/src/main/java/org/apache/kafka/common/metrics/stats/Rate.ja

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-14 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/#review95496 --- Ship it! Minor edits that I will take care of on check-in. core/s

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-14 Thread Aditya Auradkar
> On Aug. 14, 2015, 1:57 a.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/metrics/stats/Rate.java, > > lines 69-77 > > > > > > This is probably not the right place to throw QuotaViolationExcept

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-14 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated Aug. 15, 2015, 12:43 a.m.) Review request for kafka, Joel Koshy and Ju

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-14 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated Aug. 15, 2015, 12:43 a.m.) Review request for kafka, Joel Koshy and Ju

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-14 Thread Joel Koshy
> On Aug. 14, 2015, 1:57 a.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/metrics/stats/Rate.java, > > lines 69-77 > > > > > > This is probably not the right place to throw QuotaViolationExcept

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-13 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated Aug. 14, 2015, 2:20 a.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-13 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated Aug. 14, 2015, 2:19 a.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-13 Thread Aditya Auradkar
> On Aug. 14, 2015, 1:57 a.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/metrics/stats/Rate.java, > > lines 69-77 > > > > > > This is probably not the right place to throw QuotaViolationExcept

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-13 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated Aug. 14, 2015, 2:09 a.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-13 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated Aug. 14, 2015, 2:08 a.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-13 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/#review95369 --- clients/src/main/java/org/apache/kafka/common/metrics/stats/Rate.ja

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-13 Thread Joel Koshy
> On Aug. 13, 2015, 11:13 p.m., Joel Koshy wrote: > > build.gradle, line 383 > > > > > > I don't think these changes 383-385 are needed. > > > > Also, (unrelated to this change) I'm seeing NPE errors in run

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-13 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/#review95344 --- build.gradle (line 383)

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-12 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated Aug. 13, 2015, 4:25 a.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-12 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated Aug. 13, 2015, 4:24 a.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-12 Thread Joel Koshy
> On Aug. 6, 2015, 4:17 p.m., Jun Rao wrote: > > A few more comments. > > > > We need to be careful with sensors at the client-id level. Clients can come > > and go (e.g. console consumer). We probably don't want to hold sensors that > > are not longer actively used since it takes memory. So,

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-12 Thread Joel Koshy
> On June 17, 2015, 4:40 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/ClientQuotaMetrics.scala, line 1 > > > > > > Why was MockTime moved from test to main? > > Aditya Auradkar wrote: > Because I ne

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-12 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/#review95216 --- core/src/main/scala/kafka/server/ClientQuotaManager.scala (line 65)

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-12 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated Aug. 12, 2015, 7:09 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-12 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated Aug. 12, 2015, 7:08 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-12 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated Aug. 12, 2015, 7:05 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-12 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated Aug. 12, 2015, 7:04 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-12 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated Aug. 12, 2015, 7:03 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-12 Thread Aditya Auradkar
> On Aug. 12, 2015, 12:34 a.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/metrics/Sensor.java, lines > > 131-150 > > > > > > I think the comment can be a simpler. Basically, if O is the obser

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-12 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated Aug. 12, 2015, 7:02 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-12 Thread Aditya Auradkar
> On Aug. 12, 2015, 12:42 a.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/KafkaConfig.scala, line 419 > > > > > > I am still not sure that I see the value of the delay factor. If one > > wants to be a bit

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-11 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/#review95035 --- core/src/main/scala/kafka/server/KafkaConfig.scala (line 418)

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-11 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/#review95033 --- Just a couple of comments below. Otherwise, LGTM. clients/src/main

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-10 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated Aug. 11, 2015, 4:58 a.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-10 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated Aug. 11, 2015, 4:57 a.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-10 Thread Aditya Auradkar
> On Aug. 6, 2015, 2:02 a.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/metrics/Sensor.java, lines > > 135-139 > > > > > > Is that calculation here right? Based on the calculation in Throttle

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-10 Thread Aditya Auradkar
> On Aug. 6, 2015, 4:17 p.m., Jun Rao wrote: > > A few more comments. > > > > We need to be careful with sensors at the client-id level. Clients can come > > and go (e.g. console consumer). We probably don't want to hold sensors that > > are not longer actively used since it takes memory. So,

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-10 Thread Jun Rao
> On Aug. 6, 2015, 4:17 p.m., Jun Rao wrote: > > A few more comments. > > > > We need to be careful with sensors at the client-id level. Clients can come > > and go (e.g. console consumer). We probably don't want to hold sensors that > > are not longer actively used since it takes memory. So,

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-10 Thread Jun Rao
> On Aug. 6, 2015, 2:02 a.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/metrics/Sensor.java, lines > > 135-139 > > > > > > Is that calculation here right? Based on the calculation in Throttle

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-10 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated Aug. 10, 2015, 8:49 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-10 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated Aug. 10, 2015, 8:48 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-07 Thread Aditya Auradkar
> On Aug. 6, 2015, 2:02 a.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/metrics/Sensor.java, lines > > 135-139 > > > > > > Is that calculation here right? Based on the calculation in Throttle

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-07 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated Aug. 7, 2015, 6:28 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-07 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated Aug. 7, 2015, 6:27 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-07 Thread Aditya Auradkar
> On Aug. 6, 2015, 4:17 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/ClientQuotaManager.scala, lines 154-201 > > > > > > Not sure why the lock is needed. metrics.sensor() is synchronized and > > alreay

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-07 Thread Aditya Auradkar
> On Aug. 6, 2015, 2:02 a.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/ClientQuotaManager.scala, lines 208-211 > > > > > > Instead of creating a new metric instance, we probably should just > > reuse the

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-06 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/#review94412 --- A few more comments. We need to be careful with sensors at the clie

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-05 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/#review94333 --- Thanks for the patch. A few comments below. clients/src/main/java/

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-04 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated Aug. 5, 2015, 2:08 a.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-04 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated Aug. 5, 2015, 2:07 a.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-04 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated Aug. 5, 2015, 1:51 a.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-04 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated Aug. 5, 2015, 1:50 a.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-04 Thread Aditya Auradkar
> On Aug. 4, 2015, 12:36 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/ClientQuotaMetrics.scala, line 98 > > > > > > A number of places in this patch use a : b instead of a: b. Highly > > stylistic, I'm

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-04 Thread Aditya Auradkar
> On Aug. 4, 2015, 3:28 a.m., Edward Ribeiro wrote: > > clients/src/main/java/org/apache/kafka/common/metrics/QuotaViolationException.java, > > line 27 > > > > > > It's a very good pratice to make any field ``final``

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-03 Thread Edward Ribeiro
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/#review93991 --- clients/src/main/java/org/apache/kafka/common/metrics/Quota.java (l

Re: Review Request 33049: Patch for KAFKA-2084

2015-08-03 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/#review93965 --- Looks good overall. I have a few more minor comments/suggestions. Ap

Re: Review Request 33049: Patch for KAFKA-2084

2015-06-29 Thread Aditya Auradkar
> On June 17, 2015, 4:40 p.m., Joel Koshy wrote: > > core/src/test/scala/integration/kafka/api/QuotasTest.scala, line 142 > > > > > > This is an important test, but this is a bit non-deterministic no? > > i.e., the r

Re: Review Request 33049: Patch for KAFKA-2084

2015-06-29 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated June 30, 2015, 12:54 a.m.) Review request for kafka, Joel Koshy and Ju

Re: Review Request 33049: Patch for KAFKA-2084

2015-06-29 Thread Aditya Auradkar
> On June 17, 2015, 4:40 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/ClientQuotaMetrics.scala, line 1 > > > > > > Why was MockTime moved from test to main? Because I need to depend on MockTime from cli

Re: Review Request 33049: Patch for KAFKA-2084

2015-06-29 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated June 30, 2015, 12:53 a.m.) Review request for kafka, Joel Koshy and Ju

Re: Review Request 33049: Patch for KAFKA-2084

2015-06-17 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/#review88180 --- This is looking very good. clients/src/main/java/org/apache/kafka/

Re: Review Request 33049: Patch for KAFKA-2084

2015-06-12 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated June 12, 2015, 5:40 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-06-12 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated June 12, 2015, 5:39 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-06-04 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated June 4, 2015, 11:32 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-06-04 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated June 4, 2015, 11:31 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-06-04 Thread Aditya Auradkar
> On June 3, 2015, 3:57 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/KafkaServer.scala, line 200 > > > > > > Can we consider migrating the server to use the Time interface in > > clients and just use

Re: Review Request 33049: Patch for KAFKA-2084

2015-06-03 Thread Aditya Auradkar
> On May 28, 2015, 12:20 a.m., Dong Lin wrote: > > core/src/main/scala/kafka/server/ClientQuotaMetrics.scala, line 60 > > > > > > Should we set this to 0, or remove this configuratrion, if there is no > > use case for

Re: Review Request 33049: Patch for KAFKA-2084

2015-06-03 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/#review86418 --- I had collected my comments on this older revision. I have not looke

Re: Review Request 33049: Patch for KAFKA-2084

2015-06-02 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated June 3, 2015, 12:16 a.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-06-02 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated June 3, 2015, 12:10 a.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-06-02 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated June 3, 2015, 12:09 a.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-06-02 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated June 3, 2015, 12:02 a.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-05-28 Thread Dong Lin
> On May 28, 2015, 12:58 p.m., Manikumar Reddy O wrote: > > core/src/main/scala/kafka/server/ClientQuotaMetrics.scala, line 145 > > > > > > Are we using clientID as uniqueKey?. But as of now, clientID is not > > mand

Re: Review Request 33049: Patch for KAFKA-2084

2015-05-28 Thread Manikumar Reddy O
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/#review85545 --- core/src/main/scala/kafka/server/ClientQuotaMetrics.scala

Re: Review Request 33049: Patch for KAFKA-2084

2015-05-27 Thread Dong Lin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/#review85465 --- core/src/main/scala/kafka/server/ClientQuotaMetrics.scala

Re: Review Request 33049: Patch for KAFKA-2084

2015-05-26 Thread Aditya Auradkar
> On May 12, 2015, 7:38 p.m., Dong Lin wrote: > > clients/src/main/java/org/apache/kafka/common/metrics/Sensor.java, line 117 > > > > > > metric.value(timeMs), which translates to Rate.measure(config, timeMs), > > may

Re: Review Request 33049: Patch for KAFKA-2084

2015-05-26 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated May 26, 2015, 6:53 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-05-26 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated May 26, 2015, 6:50 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-05-17 Thread Dong Lin
> On May 12, 2015, 7:38 p.m., Dong Lin wrote: > > clients/src/main/java/org/apache/kafka/common/metrics/Sensor.java, lines > > 122-127 > > > > > > The function quits on the first quota violaation, and calculate > > d

Re: Review Request 33049: Patch for KAFKA-2084

2015-05-15 Thread Dong Lin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/#review84025 --- core/src/main/scala/kafka/server/KafkaApis.scala

Re: Review Request 33049: Patch for KAFKA-2084

2015-05-12 Thread Aditya Auradkar
> On May 12, 2015, 7:38 p.m., Dong Lin wrote: > > clients/src/main/java/org/apache/kafka/common/metrics/Sensor.java, line 117 > > > > > > metric.value(timeMs), which translates to Rate.measure(config, timeMs), > > may

Re: Review Request 33049: Patch for KAFKA-2084

2015-05-12 Thread Dong Lin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/#review83441 --- clients/src/main/java/org/apache/kafka/common/metrics/Sensor.java <

Re: Review Request 33049: Patch for KAFKA-2084

2015-05-11 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated May 11, 2015, 11:17 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-05-11 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated May 11, 2015, 11:16 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-05-05 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated May 6, 2015, 12:52 a.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-05-05 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated May 6, 2015, 12:52 a.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-05-05 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated May 5, 2015, 10:29 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-05-05 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated May 5, 2015, 10:27 p.m.) Review request for kafka, Joel Koshy and Jun

Re: Review Request 33049: Patch for KAFKA-2084

2015-05-04 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated May 5, 2015, 4:36 a.m.) Review request for kafka, Joel Koshy and Jun R

Re: Review Request 33049: Patch for KAFKA-2084

2015-05-04 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated May 5, 2015, 4:33 a.m.) Review request for kafka, Joel Koshy and Jun R

Re: Review Request 33049: Patch for KAFKA-2084

2015-05-04 Thread Aditya Auradkar
> On May 4, 2015, 3:46 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/ClientQuotaMetrics2.scala, lines 153-162 > > > > > > For measuring the amount of throtting, would it be better to measure it > > as a pe

Re: Review Request 33049: Patch for KAFKA-2084

2015-05-04 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/#review82382 --- Thanks for the patch. I agree that ClientQuotaMetrics2 is better.

Re: Review Request 33049: Patch for KAFKA-2084

2015-04-21 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated April 21, 2015, 7:33 p.m.) Review request for kafka and Joel Koshy.

Re: Review Request 33049: Patch for KAFKA-2084

2015-04-21 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated April 21, 2015, 7:28 p.m.) Review request for kafka and Joel Koshy.

Re: Review Request 33049: Patch for KAFKA-2084

2015-04-21 Thread Aditya Auradkar
> On April 17, 2015, 11:21 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/ClientQuotaMetrics.scala, line 33 > > > > > > Is this necessary? Not strictly but I felt it was a nice to have. I can make the def

Re: Review Request 33049: Patch for KAFKA-2084

2015-04-21 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated April 21, 2015, 7:21 p.m.) Review request for kafka and Joel Koshy.

Re: Review Request 33049: Patch for KAFKA-2084

2015-04-17 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/#review80130 --- clients/src/main/java/org/apache/kafka/common/metrics/MetricConfig.

Re: Review Request 33049: Patch for KAFKA-2084

2015-04-10 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated April 11, 2015, 12:25 a.m.) Review request for kafka. Bugs: KAFKA-20

Re: Review Request 33049: Patch for KAFKA-2084

2015-04-10 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/ --- (Updated April 11, 2015, 12:24 a.m.) Review request for kafka. Bugs: KAFKA-20