-----------------------------------------------------------
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.java (lines 69 
- 77)
<https://reviews.apache.org/r/33049/#comment150316>

    This is probably not the right place to throw QuotaViolationException. 
Rate.measure() can be called when the JMX value is polled and we don't want to 
throw an exception there.
    
    I was thinking that another way to do this is to perhaps add the logic in 
Quota.acceptable(). We can probably change it to be
    
    // if the check passes, return -1; otherwise return a delay to make the 
check pass.
    int Quota.check(KafkaMetrics, long now).
    
    In the implementation, we can then check if the KafkaMetrics passed in has 
a measurable that's an instance of Rate. If so, we will compute the delay for 
rate (the window should be elapsedCurrent and elapsedPrior as computed in 
Rate.measure()).


- Jun Rao


On Aug. 13, 2015, 4:25 a.m., Aditya Auradkar wrote:
> 
> -----------------------------------------------------------
> 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 Rao.
> 
> 
> Bugs: KAFKA-2084
>     https://issues.apache.org/jira/browse/KAFKA-2084
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Updated patch for quotas. This patch does the following: 
> 1. Add per-client metrics for both producer and consumers 
> 2. Add configuration for quotas 
> 3. Compute delay times in the metrics package and return the delay times in 
> QuotaViolationException 
> 4. Add a DelayQueue in KafkaApi's that can be used to throttle any type of 
> request. Implemented request throttling for produce and fetch requests. 
> 5. Added unit and integration test cases for both producer and consumer
> 6. This doesn't include a system test. There is a separate ticket for that
> 7. Fixed KAFKA-2191 - (Included fix from : 
> https://reviews.apache.org/r/34418/ )
> 
> Addressed comments from Joel and Jun
> 
> 
> Diffs
> -----
> 
>   build.gradle 1b67e628c2fca897177c12b6afad9a8700fffd1f 
>   clients/src/main/java/org/apache/kafka/common/metrics/Quota.java 
> d82bb0c055e631425bc1ebbc7d387baac76aeeaa 
>   
> clients/src/main/java/org/apache/kafka/common/metrics/QuotaViolationException.java
>  a451e5385c9eca76b38b425e8ac856b2715fcffe 
>   clients/src/main/java/org/apache/kafka/common/metrics/Sensor.java 
> ca823fd4639523018311b814fde69b6177e73b97 
>   clients/src/main/java/org/apache/kafka/common/metrics/stats/Rate.java 
> 98429da34418f7f1deba1b5e44e2e6025212edb3 
>   clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java 
> 544e120594de78c43581a980b1e4087b4fb98ccb 
>   core/src/main/scala/kafka/server/ClientQuotaManager.scala PRE-CREATION 
>   core/src/main/scala/kafka/server/KafkaApis.scala 
> 7ea509c2c41acc00430c74e025e069a833aac4e7 
>   core/src/main/scala/kafka/server/KafkaConfig.scala 
> dbe170f87331f43e2dc30165080d2cb7dfe5fdbf 
>   core/src/main/scala/kafka/server/KafkaServer.scala 
> 84d4730ac634f9a5bf12a656e422fea03ad72da8 
>   core/src/main/scala/kafka/server/ReplicaManager.scala 
> 795220e7f63d163be90738b4c1a39687b44c1395 
>   core/src/main/scala/kafka/server/ThrottledResponse.scala PRE-CREATION 
>   core/src/main/scala/kafka/utils/ShutdownableThread.scala 
> fc226c863095b7761290292cd8755cd7ad0f155c 
>   core/src/test/scala/integration/kafka/api/QuotasTest.scala PRE-CREATION 
>   core/src/test/scala/unit/kafka/server/ClientQuotaManagerTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 
> f32d206d3f52f3f9f4d649c213edd7058f4b6150 
>   core/src/test/scala/unit/kafka/server/ThrottledResponseExpirationTest.scala 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/33049/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Aditya Auradkar
> 
>

Reply via email to