> On April 17, 2015, 11:21 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/ClientQuotaMetrics.scala, line 33 > > <https://reviews.apache.org/r/33049/diff/3/?file=924194#file924194line33> > > > > Is this necessary?
Not strictly but I felt it was a nice to have. I can make the default 1 if it helps. I'm envisioning a scenario where we discover in production that our throttle time computation isn't agressive enough and this can help in such a case. > On April 17, 2015, 11:21 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/ClientQuotaMetrics.scala, line 81 > > <https://reviews.apache.org/r/33049/diff/3/?file=924194#file924194line81> > > > > Overall, I think this should work, but it seems slightly high touch no? > > > > i.e., do we really need to wrap everything? i.e., you definitely need a > > clientQuotaMetricsConfig, but it seems many of the wrapper routines here > > can be folded into the core metrics package. > > > > E.g., otherwise for every metric that we want to quota on, we are > > forced to add new record* methods; > > > > If I'm reading this right, a motivation for having the wrappers is to > > getOrCreate the sensors. Can we just pre-emptively (at the beginning) > > create the per-client sensors and then avoid the wrapper routines? This > > would also help avoid the need for the extra quota map and the > > synchronization logic in creating the sensors. It isn't possible to pro-actively create all the sensors since we don't know in advance what clients will connect to the system. Eventually, when we have dynamic configs we can't even proactively create sensors for the overridden ones. I've published a new patch that will make this easier to use i.e. not have typed methods per metric we want to throttle. - Aditya ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33049/#review80130 ----------------------------------------------------------- On April 21, 2015, 7:21 p.m., Aditya Auradkar wrote: > > ----------------------------------------------------------- > 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. > > > Bugs: KAFKA-2084 > https://issues.apache.org/jira/browse/KAFKA-2084 > > > Repository: kafka > > > Description > ------- > > This is currently not being used anywhere in the code because I haven't yet > figured out how to enforce delays i.e. purgatory vs delay queue. I'll have a > better idea once I look at the new purgatory implementation. Hopefully, this > smaller patch is easier to review. > > Added more testcases > > > Some locking changes for reading/creating the sensors > > > WIP patch > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/common/metrics/MetricConfig.java > dfa1b0a11042ad9d127226f0e0cec8b1d42b8441 > clients/src/main/java/org/apache/kafka/common/metrics/Metrics.java > b3d3d7c56acb445be16a3fbe00f05eaba659be46 > 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 > core/src/main/scala/kafka/server/ClientQuotaMetrics.scala PRE-CREATION > core/src/main/scala/kafka/server/KafkaConfig.scala > 69b772c1941865fbe15b34bb2784c511f8ce519a > core/src/main/scala/kafka/server/KafkaServer.scala > c63f4ba9d622817ea8636d4e6135fba917ce085a > core/src/main/scala/kafka/server/ReplicaManager.scala > 8ddd325015de4245fd2cf500d8b0e8c1fd2bc7e8 > core/src/test/scala/unit/kafka/server/ClientQuotaMetricsTest.scala > PRE-CREATION > > Diff: https://reviews.apache.org/r/33049/diff/ > > > Testing > ------- > > > Thanks, > > Aditya Auradkar > >
