----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33557/#review82392 -----------------------------------------------------------
This looks good overall. Couple of questions: - You have blacklisted the topic for recording in the global broker topic metrics, but for consistency should we do this for per-topic metrics as well? - Rather than do the topic check in various places (which is error-prone) perhaps we can make BrokerTopicMetric accept a blacklist of topics (currently on offsets topic) and have setter methods (e.g., bytesIn, messagesOut, etc.) which will proceed to record the metric only if topic is not in blacklist. Another approach would be to maintain two sets of broker-topic-metrics. One for internal topics, one for other topics. The FailedProduce metrics should really have been called failed append. - Joel Koshy On April 27, 2015, 4:26 a.m., Dong Lin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33557/ > ----------------------------------------------------------- > > (Updated April 27, 2015, 4:26 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1936 > https://issues.apache.org/jira/browse/KAFKA-1936 > > > Repository: kafka > > > Description > ------- > > KAFKA-1936; Track offset commit requests separately from produce requests > > > Diffs > ----- > > core/src/main/scala/kafka/log/Log.scala > 84e7b8fe9dd014884b60c4fbe13c835cf02a40e4 > core/src/main/scala/kafka/server/KafkaApis.scala > b4004aa3a1456d337199aa1245fb0ae61f6add46 > core/src/main/scala/kafka/server/ReplicaManager.scala > 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 > core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala > 652208a70f66045b854549d93cbbc2b77c24b10b > > Diff: https://reviews.apache.org/r/33557/diff/ > > > Testing > ------- > > > Thanks, > > Dong Lin > >
