> On July 14, 2015, 1:59 a.m., Joel Koshy wrote:
> > core/src/main/scala/kafka/common/AppInfo.scala, line 27
> > <https://reviews.apache.org/r/35867/diff/3/?file=1004947#file1004947line27>
> >
> >     Per the comment in the previous diff, I think this can go now right? 
> > i.e., kafka server depends on clients so if you browse mbeans you will see 
> > two app-infos registered (under `kafka.server` and `kafka.common`) which is 
> > weird. The server will also expose app-info via the clients package since 
> > it already uses kafka metrics and the associated jmx reporter.

Old AppInfo registration is removed. Now on Kafka Server, we will only see one 
app-info mbean.


> On July 14, 2015, 1:59 a.m., Joel Koshy wrote:
> > clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java, 
> > line 27
> > <https://reviews.apache.org/r/35867/diff/3/?file=1004944#file1004944line27>
> >
> >     (For consistency) should we make this 40-char wide as is a standard 
> > commit id? Or we can just go with a eight-char or 16-char wide id for both 
> > this and the actual commit id.

Ok..I am going with 16 char id.


> On July 14, 2015, 1:59 a.m., Joel Koshy wrote:
> > clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java, 
> > line 246
> > <https://reviews.apache.org/r/35867/diff/3/?file=1004943#file1004943line246>
> >
> >     In 
> > https://issues.apache.org/jira/browse/KAFKA-1901?focusedCommentId=14294803&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14294803
> >  I was wondering if we could have a commit fingerprint - i.e., the long 
> > value of the most-significant eight bytes of the commit modulo 10k or 
> > something like that. This would make is convenient to register as a 
> > measurable `KafkaMetric` that people can then use in their deployment 
> > monitoring. i.e., instantly look at a graph and say whether all 
> > brokers/clients are running the same version or not.

Implemented a simple finger print (commitId.hashcode()) mechanism. I felt 
commit.hashCode() should be sufficient for our requirement.


- Manikumar Reddy


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35867/#review91574
-----------------------------------------------------------


On July 14, 2015, 12:32 p.m., Manikumar Reddy O wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35867/
> -----------------------------------------------------------
> 
> (Updated July 14, 2015, 12:32 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1901
>     https://issues.apache.org/jira/browse/KAFKA-1901
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Addresing Joel's comments
> 
> 
> Diffs
> -----
> 
>   build.gradle d86f1a8b25197d53f11e16c54a6854487e175649 
>   clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 
> b4e8f7f0dceefaf94a3495f39f5783cce5ceb25f 
>   clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 
> 03b8dd23df63a8d8a117f02eabcce4a2d48c44f7 
>   clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java 
> 6b9590c418aedd2727544c5dd23c017b4b72467a 
>   clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java 
> PRE-CREATION 
>   clients/src/test/java/org/apache/kafka/common/metrics/JmxReporterTest.java 
> 07b1b60d3a9cb1a399a2fe95b87229f64f539f3b 
>   clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java 
> 544e120594de78c43581a980b1e4087b4fb98ccb 
>   core/src/main/scala/kafka/common/AppInfo.scala 
> d642ca555f83c41451d4fcaa5c01a1f86eff0a1c 
>   core/src/main/scala/kafka/server/KafkaServer.scala 
> 18917bc4464b9403b16d85d20c3fd4c24893d1d3 
>   core/src/main/scala/kafka/server/KafkaServerStartable.scala 
> 1c1b75b4137a8b233b61739018e9cebcc3a34343 
> 
> Diff: https://reviews.apache.org/r/35867/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Manikumar Reddy O
> 
>

Reply via email to