You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Manikumar Reddy O <ma...@gmail.com> on 2015/07/10 13:15:37 UTC
Re: Review Request 35867: Patch for KAFKA-1901
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35867/
-----------------------------------------------------------
(Updated July 10, 2015, 11:15 a.m.)
Review request for kafka.
Bugs: KAFKA-1901
https://issues.apache.org/jira/browse/KAFKA-1901
Repository: kafka
Description (updated)
-------
patch after rebase
Diffs (updated)
-----
build.gradle d86f1a8b25197d53f11e16c54a6854487e175649
clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 7aa076084c894bb8f47b9df2c086475b06f47060
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
Diff: https://reviews.apache.org/r/35867/diff/
Testing
-------
Thanks,
Manikumar Reddy O
Re: Review Request 35867: Patch for KAFKA-1901
Posted by Manikumar Reddy O <ma...@gmail.com>.
> 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
>
>
Re: Review Request 35867: Patch for KAFKA-1901
Posted by Joel Koshy <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35867/#review91574
-----------------------------------------------------------
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java (line 207)
<https://reviews.apache.org/r/35867/#comment145018>
`jmxPrefix, clientId`
clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java (line 152)
<https://reviews.apache.org/r/35867/#comment145010>
`log.warn("error...", e)` - also, whitespace before `"`
clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java (line 164)
<https://reviews.apache.org/r/35867/#comment145011>
same
clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java (line 246)
<https://reviews.apache.org/r/35867/#comment145023>
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.
clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java (line 27)
<https://reviews.apache.org/r/35867/#comment145019>
(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.
clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java (line 33)
<https://reviews.apache.org/r/35867/#comment145020>
should probably reference the above constants (ln 26, 27) here instead of hardcoding again.
core/src/main/scala/kafka/common/AppInfo.scala (line 24)
<https://reviews.apache.org/r/35867/#comment145021>
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.
- Joel Koshy
On July 10, 2015, 11:15 a.m., Manikumar Reddy O wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35867/
> -----------------------------------------------------------
>
> (Updated July 10, 2015, 11:15 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1901
> https://issues.apache.org/jira/browse/KAFKA-1901
>
>
> Repository: kafka
>
>
> Description
> -------
>
> patch after rebase
>
>
> Diffs
> -----
>
> build.gradle d86f1a8b25197d53f11e16c54a6854487e175649
> clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 7aa076084c894bb8f47b9df2c086475b06f47060
> 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
>
> Diff: https://reviews.apache.org/r/35867/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Manikumar Reddy O
>
>
Re: Review Request 35867: Patch for KAFKA-1901
Posted by Manikumar Reddy O <ma...@gmail.com>.
> On July 17, 2015, 4:25 p.m., Joel Koshy wrote:
> > clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java, line 56
> > <https://reviews.apache.org/r/35867/diff/4/?file=1011316#file1011316line56>
> >
> > "given `prefix` string and further qualifies the associated AppInfo mbean with the attribute `id`."
> >
> > Actually, sorry to bring this up again (we discussed it briefly in the first diff), but I think it would be better to register the app info explicitly in the consumer/producer/broker and remove this argument. It is convenient to have it instantiated as part of JmxReporter, but it is weird to have to pass in an `id` that will be used only for the app-info mbean.
done the required changes to explicitly register the app info in the consumer/producer/broker.
- Manikumar Reddy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35867/#review92078
-----------------------------------------------------------
On Aug. 9, 2015, 9:37 a.m., Manikumar Reddy O wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35867/
> -----------------------------------------------------------
>
> (Updated Aug. 9, 2015, 9:37 a.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 1b67e628c2fca897177c12b6afad9a8700fffd1f
> clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java ed99e9bdf7c4ea7a6d4555d4488cf8ed0b80641b
> clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 03b8dd23df63a8d8a117f02eabcce4a2d48c44f7
> clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java PRE-CREATION
> core/src/main/scala/kafka/common/AppInfo.scala d642ca555f83c41451d4fcaa5c01a1f86eff0a1c
> core/src/main/scala/kafka/server/KafkaServer.scala 84d4730ac634f9a5bf12a656e422fea03ad72da8
> core/src/main/scala/kafka/server/KafkaServerStartable.scala 1c1b75b4137a8b233b61739018e9cebcc3a34343
>
> Diff: https://reviews.apache.org/r/35867/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Manikumar Reddy O
>
>
Re: Review Request 35867: Patch for KAFKA-1901
Posted by Joel Koshy <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35867/#review92078
-----------------------------------------------------------
clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java (line 56)
<https://reviews.apache.org/r/35867/#comment146010>
"given `prefix` string and further qualifies the associated AppInfo mbean with the attribute `id`."
Actually, sorry to bring this up again (we discussed it briefly in the first diff), but I think it would be better to register the app info explicitly in the consumer/producer/broker and remove this argument. It is convenient to have it instantiated as part of JmxReporter, but it is weird to have to pass in an `id` that will be used only for the app-info mbean.
clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java (line 148)
<https://reviews.apache.org/r/35867/#comment146037>
For consistency with our regular metrics registered via KafkaMetrics maybe we should make this `app-info`
clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java (line 247)
<https://reviews.apache.org/r/35867/#comment146041>
(for consistency) `getCommitId`
Also, `getCommitFingerprint` (fp is one word)
Also, how about these as alternates:
`getCommit` and `getCommitHash`
clients/src/main/java/org/apache/kafka/common/metrics/JmxReporter.java (line 254)
<https://reviews.apache.org/r/35867/#comment146038>
Can we also log the commit id and fingerprint?
- Joel Koshy
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
>
>
Re: Review Request 35867: Patch for KAFKA-1901
Posted by Joel Koshy <jj...@gmail.com>.
> On July 21, 2015, 1:58 p.m., Ismael Juma wrote:
> > clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java, line 27
> > <https://reviews.apache.org/r/35867/diff/4/?file=1011317#file1011317line27>
> >
> > Why isn't this "unknown" like `version`?
>
> Manikumar Reddy O wrote:
> yes we can set "unknown" here. Joel suggested to use 00000000 as the unknown commit ID. Joel, can we change this?
Sure - I originally thought we could make the fingerprint == the numeric value associated with the leading bytes. That would have made it easy to just take the fingerprint -> convert to hex and get the git hash prefix. However, I think we can just drop the fingerprint altogether for now.
- Joel
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35867/#review92404
-----------------------------------------------------------
On Aug. 9, 2015, 9:37 a.m., Manikumar Reddy O wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35867/
> -----------------------------------------------------------
>
> (Updated Aug. 9, 2015, 9:37 a.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 1b67e628c2fca897177c12b6afad9a8700fffd1f
> clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java ed99e9bdf7c4ea7a6d4555d4488cf8ed0b80641b
> clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 03b8dd23df63a8d8a117f02eabcce4a2d48c44f7
> clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java PRE-CREATION
> core/src/main/scala/kafka/common/AppInfo.scala d642ca555f83c41451d4fcaa5c01a1f86eff0a1c
> core/src/main/scala/kafka/server/KafkaServer.scala 84d4730ac634f9a5bf12a656e422fea03ad72da8
> core/src/main/scala/kafka/server/KafkaServerStartable.scala 1c1b75b4137a8b233b61739018e9cebcc3a34343
>
> Diff: https://reviews.apache.org/r/35867/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Manikumar Reddy O
>
>
Re: Review Request 35867: Patch for KAFKA-1901
Posted by Manikumar Reddy O <ma...@gmail.com>.
> On July 21, 2015, 1:58 p.m., Ismael Juma wrote:
> > clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java, line 27
> > <https://reviews.apache.org/r/35867/diff/4/?file=1011317#file1011317line27>
> >
> > Why isn't this "unknown" like `version`?
yes we can set "unknown" here. Joel suggested to use 00000000 as the unknown commit ID. Joel, can we change this?
- Manikumar Reddy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35867/#review92404
-----------------------------------------------------------
On Aug. 9, 2015, 9:37 a.m., Manikumar Reddy O wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35867/
> -----------------------------------------------------------
>
> (Updated Aug. 9, 2015, 9:37 a.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 1b67e628c2fca897177c12b6afad9a8700fffd1f
> clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java ed99e9bdf7c4ea7a6d4555d4488cf8ed0b80641b
> clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 03b8dd23df63a8d8a117f02eabcce4a2d48c44f7
> clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java PRE-CREATION
> core/src/main/scala/kafka/common/AppInfo.scala d642ca555f83c41451d4fcaa5c01a1f86eff0a1c
> core/src/main/scala/kafka/server/KafkaServer.scala 84d4730ac634f9a5bf12a656e422fea03ad72da8
> core/src/main/scala/kafka/server/KafkaServerStartable.scala 1c1b75b4137a8b233b61739018e9cebcc3a34343
>
> Diff: https://reviews.apache.org/r/35867/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Manikumar Reddy O
>
>
Re: Review Request 35867: Patch for KAFKA-1901
Posted by Ismael Juma <mi...@juma.me.uk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35867/#review92404
-----------------------------------------------------------
build.gradle (line 388)
<https://reviews.apache.org/r/35867/#comment146572>
Would it not be better to leave this value unset if we can't figure it out?
clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java (line 27)
<https://reviews.apache.org/r/35867/#comment146573>
Why isn't this "unknown" like `version`?
- Ismael Juma
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
>
>
Re: Review Request 35867: Patch for KAFKA-1901
Posted by Joel Koshy <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35867/#review96003
-----------------------------------------------------------
Ship it!
Ship It!
- Joel Koshy
On Aug. 20, 2015, 7:08 a.m., Manikumar Reddy O wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35867/
> -----------------------------------------------------------
>
> (Updated Aug. 20, 2015, 7:08 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1901
> https://issues.apache.org/jira/browse/KAFKA-1901
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Addresing Joel's comments, rebase
>
>
> Diffs
> -----
>
> build.gradle 17fc223907f6b55f2d730be81ddb8d8e07a2c7ad
> clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 3749880b765f74af117d6c44705daf170095a1b7
> clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java c4621e22c32c1a1fb23726d7f56004845def96ef
> clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java PRE-CREATION
> core/src/main/scala/kafka/common/AppInfo.scala d642ca555f83c41451d4fcaa5c01a1f86eff0a1c
> core/src/main/scala/kafka/server/KafkaServer.scala 0e7ba3ede78dbc995c404e0387a6be687703836a
> core/src/main/scala/kafka/server/KafkaServerStartable.scala 1c1b75b4137a8b233b61739018e9cebcc3a34343
>
> Diff: https://reviews.apache.org/r/35867/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Manikumar Reddy O
>
>
Re: Review Request 35867: Patch for KAFKA-1901
Posted by Manikumar Reddy O <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35867/
-----------------------------------------------------------
(Updated Aug. 20, 2015, 7:08 a.m.)
Review request for kafka.
Bugs: KAFKA-1901
https://issues.apache.org/jira/browse/KAFKA-1901
Repository: kafka
Description (updated)
-------
Addresing Joel's comments, rebase
Diffs (updated)
-----
build.gradle 17fc223907f6b55f2d730be81ddb8d8e07a2c7ad
clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 3749880b765f74af117d6c44705daf170095a1b7
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java c4621e22c32c1a1fb23726d7f56004845def96ef
clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java PRE-CREATION
core/src/main/scala/kafka/common/AppInfo.scala d642ca555f83c41451d4fcaa5c01a1f86eff0a1c
core/src/main/scala/kafka/server/KafkaServer.scala 0e7ba3ede78dbc995c404e0387a6be687703836a
core/src/main/scala/kafka/server/KafkaServerStartable.scala 1c1b75b4137a8b233b61739018e9cebcc3a34343
Diff: https://reviews.apache.org/r/35867/diff/
Testing
-------
Thanks,
Manikumar Reddy O
Re: Review Request 35867: Patch for KAFKA-1901
Posted by Manikumar Reddy O <ma...@gmail.com>.
> On Aug. 20, 2015, 2:03 a.m., Joel Koshy wrote:
> > build.gradle, line 389
> > <https://reviews.apache.org/r/35867/diff/5/?file=1035356#file1035356line389>
> >
> > This gives an error in detached mode (i.e., not on any branch).
Updated code to handle detached mode. thanks for the review.
- Manikumar Reddy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35867/#review95902
-----------------------------------------------------------
On Aug. 20, 2015, 7:08 a.m., Manikumar Reddy O wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35867/
> -----------------------------------------------------------
>
> (Updated Aug. 20, 2015, 7:08 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1901
> https://issues.apache.org/jira/browse/KAFKA-1901
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Addresing Joel's comments, rebase
>
>
> Diffs
> -----
>
> build.gradle 17fc223907f6b55f2d730be81ddb8d8e07a2c7ad
> clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 3749880b765f74af117d6c44705daf170095a1b7
> clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java c4621e22c32c1a1fb23726d7f56004845def96ef
> clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java PRE-CREATION
> core/src/main/scala/kafka/common/AppInfo.scala d642ca555f83c41451d4fcaa5c01a1f86eff0a1c
> core/src/main/scala/kafka/server/KafkaServer.scala 0e7ba3ede78dbc995c404e0387a6be687703836a
> core/src/main/scala/kafka/server/KafkaServerStartable.scala 1c1b75b4137a8b233b61739018e9cebcc3a34343
>
> Diff: https://reviews.apache.org/r/35867/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Manikumar Reddy O
>
>
Re: Review Request 35867: Patch for KAFKA-1901
Posted by Joel Koshy <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35867/#review95902
-----------------------------------------------------------
Can you rebase?
build.gradle (line 389)
<https://reviews.apache.org/r/35867/#comment151107>
This gives an error in detached mode (i.e., not on any branch).
clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java (line 75)
<https://reviews.apache.org/r/35867/#comment151102>
Sorry about the misdirection here, but I think we may as well drop this fingerprint. If we want to add it later, we can - and it may be useful to take the actual numeric value of the leading eight bytes since it makes it easier to instantly associate with a commit hash (otherwise we would need to tabulate commits to their hashCode for easy lookup).
- Joel Koshy
On Aug. 9, 2015, 9:37 a.m., Manikumar Reddy O wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35867/
> -----------------------------------------------------------
>
> (Updated Aug. 9, 2015, 9:37 a.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 1b67e628c2fca897177c12b6afad9a8700fffd1f
> clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java ed99e9bdf7c4ea7a6d4555d4488cf8ed0b80641b
> clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 03b8dd23df63a8d8a117f02eabcce4a2d48c44f7
> clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java PRE-CREATION
> core/src/main/scala/kafka/common/AppInfo.scala d642ca555f83c41451d4fcaa5c01a1f86eff0a1c
> core/src/main/scala/kafka/server/KafkaServer.scala 84d4730ac634f9a5bf12a656e422fea03ad72da8
> core/src/main/scala/kafka/server/KafkaServerStartable.scala 1c1b75b4137a8b233b61739018e9cebcc3a34343
>
> Diff: https://reviews.apache.org/r/35867/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Manikumar Reddy O
>
>
Re: Review Request 35867: Patch for KAFKA-1901
Posted by Manikumar Reddy O <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35867/
-----------------------------------------------------------
(Updated Aug. 9, 2015, 9:37 a.m.)
Review request for kafka.
Bugs: KAFKA-1901
https://issues.apache.org/jira/browse/KAFKA-1901
Repository: kafka
Description
-------
Addresing Joel's comments
Diffs (updated)
-----
build.gradle 1b67e628c2fca897177c12b6afad9a8700fffd1f
clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java ed99e9bdf7c4ea7a6d4555d4488cf8ed0b80641b
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 03b8dd23df63a8d8a117f02eabcce4a2d48c44f7
clients/src/main/java/org/apache/kafka/common/utils/AppInfoParser.java PRE-CREATION
core/src/main/scala/kafka/common/AppInfo.scala d642ca555f83c41451d4fcaa5c01a1f86eff0a1c
core/src/main/scala/kafka/server/KafkaServer.scala 84d4730ac634f9a5bf12a656e422fea03ad72da8
core/src/main/scala/kafka/server/KafkaServerStartable.scala 1c1b75b4137a8b233b61739018e9cebcc3a34343
Diff: https://reviews.apache.org/r/35867/diff/
Testing
-------
Thanks,
Manikumar Reddy O
Re: Review Request 35867: Patch for KAFKA-1901
Posted by Manikumar Reddy O <ma...@gmail.com>.
-----------------------------------------------------------
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 (updated)
-------
Addresing Joel's comments
Diffs (updated)
-----
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