> On May 11, 2015, 5:25 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/KafkaRequestHandler.scala, line 147 > > <https://reviews.apache.org/r/33557/diff/4/?file=949209#file949209line147> > > > > How about > > markBrokerTopicMeters > > or > > updateBrokerTopicStats > > > > I think an even clearer approach would be to have explicit methods: > > > > ``` > > messagesIn(n) > > bytesIn(n) > > bytesOut(n) > > bytesRejected(n) > > ``` > > and so on. > > > > The current code assumes everything is a meter (which it is) but the > > above may be clearer and makes fewer assumptions about the underlying > > metric types. > > > > It may also eliminate the need for the enumeration. > > > > What do you think?
Joel: Thanks much for your suggestions. I have uploaded a patch that uses the explict methods (e.g. markMessageIn(topic, n)) for each of the eight meter in BrokerTopicMetrics, and removed enumeration. Overall the code in KafkaRequestHandler is a bit longer. But it does avoid the need of extra enumeration class and makes less assumption about the underlying metric. I think the approach is better. Thanks again. - Dong ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33557/#review83234 ----------------------------------------------------------- On May 12, 2015, 2:32 p.m., Dong Lin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33557/ > ----------------------------------------------------------- > > (Updated May 12, 2015, 2:32 p.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 > 417960dd1ab407ebebad8fdb0e97415db3e91a2f > core/src/main/scala/kafka/server/KafkaRequestHandler.scala > a1558afed20bc651ca442a774920d782890167a5 > core/src/main/scala/kafka/server/ReplicaManager.scala > 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 > core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala > 528525b719ec916e16f8b3ae3715bec4b5dcc47d > > Diff: https://reviews.apache.org/r/33557/diff/ > > > Testing > ------- > > > Thanks, > > Dong Lin > >
