> 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 > > <https://reviews.apache.org/r/33049/diff/9/?file=955824#file955824line122> > > > > The function quits on the first quota violaation, and calculate > > delayTime based on this violation. > > > > Should we enumerate all metric in this.metrics() and set delayTime of > > the QuotaViolationException to be the largest delayTime of all violatoins? > > Aditya Auradkar wrote: > Also a good point. Perhaps you can fix it in the patch you will submit? > For quotas, we effectively have only one quota per sensor, so it's ok (for > now).
Sure! Will do it. > On May 12, 2015, 7:38 p.m., Dong Lin wrote: > > clients/src/main/java/org/apache/kafka/common/metrics/Sensor.java, line 117 > > <https://reviews.apache.org/r/33049/diff/9/?file=955824#file955824line117> > > > > metric.value(timeMs), which translates to Rate.measure(config, timeMs), > > may return Infinity. This may introduce infinite delay. > > > > I think this bug is rooted either in class Rate or class SampledStat. > > In short, SampledStat.purgeObsoleteSamples will remove all Samples, such > > that SampledStat.oldest(now) == now. > > > > Should I open a ticket and submit a patch for it? > > Aditya Auradkar wrote: > Hey dong, yeah you should submit a patch for it. Sure! I will do it. - Dong ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/#review83441 ----------------------------------------------------------- On May 11, 2015, 11:17 p.m., Aditya Auradkar wrote: > > ----------------------------------------------------------- > 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 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. > 6. This doesn't include a system test. There is a separate ticket for that > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/common/metrics/MetricConfig.java > dfa1b0a11042ad9d127226f0e0cec8b1d42b8441 > 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/test/java/org/apache/kafka/common/utils/MockTime.java > core/src/main/scala/kafka/server/ClientQuotaMetrics.scala PRE-CREATION > core/src/main/scala/kafka/server/KafkaApis.scala > 417960dd1ab407ebebad8fdb0e97415db3e91a2f > core/src/main/scala/kafka/server/KafkaConfig.scala > 9efa15ca5567b295ab412ee9eea7c03eb4cdc18b > core/src/main/scala/kafka/server/KafkaServer.scala > b7d2a2842e17411a823b93bdedc84657cbd62be1 > core/src/main/scala/kafka/server/ReplicaManager.scala > 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 > core/src/main/scala/kafka/server/ThrottledRequest.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/ClientQuotaMetricsTest.scala > PRE-CREATION > core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala > 8014a5a6c362785539f24eb03d77278434614fe6 > core/src/test/scala/unit/kafka/server/ThrottledRequestExpirationTest.scala > PRE-CREATION > > Diff: https://reviews.apache.org/r/33049/diff/ > > > Testing > ------- > > > Thanks, > > Aditya Auradkar > >
