You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@samza.apache.org by Xinyu Liu <xi...@gmail.com> on 2016/07/26 18:15:27 UTC
Review Request 50451: SAMZA-981: Set consistent Kafka clientId for a
job instance
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50451/
-----------------------------------------------------------
Review request for samza, Navina Ramesh and Yi Pan (Data Infrastructure).
Repository: samza
Description
-------
SAMZA-981: Set consistent Kafka clientId for a job instance
Diffs
-----
samza-kafka/src/main/scala/org/apache/samza/config/KafkaConfig.scala 18225118ee5a3c70898274b28fe4ff0a11b55a12
samza-kafka/src/main/scala/org/apache/samza/config/RegExTopicGenerator.scala 78467bf1298653242e0b14d810e53a92a2ed139f
samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemConsumer.scala b37375360bcad8e3333089d95e8e3da63a1b9aab
samza-kafka/src/main/scala/org/apache/samza/util/KafkaUtil.scala a25ba620a58153b47059fbc11ae067f863e7c3cd
samza-kafka/src/test/scala/org/apache/samza/config/TestKafkaConfig.scala c4a83f60cf049e6fd65f940b69674895d07ab871
samza-kafka/src/test/scala/org/apache/samza/system/kafka/TestKafkaSystemConsumer.scala ece0359d1a31e65113f4832dd3baddb788cecef4
Diff: https://reviews.apache.org/r/50451/diff/
Testing
-------
gradle build & tests
Thanks,
Xinyu Liu
Re: Review Request 50451: SAMZA-981: Set consistent Kafka clientId
for a job instance
Posted by "Yi Pan (Data Infrastructure)" <yi...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50451/#review144018
-----------------------------------------------------------
Ship it!
+1. I would recommend to add the reason for this change in the JIRA title. I.e. adding " to conform to Kafka quota requirement".
samza-kafka/src/main/scala/org/apache/samza/config/RegExTopicGenerator.scala (line 102)
<https://reviews.apache.org/r/50451/#comment209967>
It would be nice to make a note here to state why we are passing in "" as clientId. Or, make the "" as a constant string w/ name like: NOT_USED_EMPTY_CONSUMER_CLIENT_ID
- Yi Pan (Data Infrastructure)
On July 26, 2016, 11:46 p.m., Xinyu Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50451/
> -----------------------------------------------------------
>
> (Updated July 26, 2016, 11:46 p.m.)
>
>
> Review request for samza, Navina Ramesh and Yi Pan (Data Infrastructure).
>
>
> Repository: samza
>
>
> Description
> -------
>
> Remove the currentTimeMills and counter in the ClientId creation so the clientId will be the same for each job instance. With this change we can turn on kafka throttling on the job instance level.
>
>
> Diffs
> -----
>
> samza-kafka/src/main/scala/org/apache/samza/config/KafkaConfig.scala 18225118ee5a3c70898274b28fe4ff0a11b55a12
> samza-kafka/src/main/scala/org/apache/samza/config/RegExTopicGenerator.scala 78467bf1298653242e0b14d810e53a92a2ed139f
> samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemConsumer.scala b37375360bcad8e3333089d95e8e3da63a1b9aab
> samza-kafka/src/main/scala/org/apache/samza/util/KafkaUtil.scala a25ba620a58153b47059fbc11ae067f863e7c3cd
> samza-kafka/src/test/scala/org/apache/samza/config/TestKafkaConfig.scala c4a83f60cf049e6fd65f940b69674895d07ab871
> samza-kafka/src/test/scala/org/apache/samza/system/kafka/TestKafkaSystemConsumer.scala ece0359d1a31e65113f4832dd3baddb788cecef4
>
> Diff: https://reviews.apache.org/r/50451/diff/
>
>
> Testing
> -------
>
> gradle build & tests
>
>
> Thanks,
>
> Xinyu Liu
>
>
Re: Review Request 50451: SAMZA-981: Set consistent Kafka clientId
for a job instance
Posted by Xinyu Liu <xi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50451/
-----------------------------------------------------------
(Updated July 26, 2016, 11:46 p.m.)
Review request for samza, Navina Ramesh and Yi Pan (Data Infrastructure).
Repository: samza
Description
-------
Remove the currentTimeMills and counter in the ClientId creation so the clientId will be the same for each job instance. With this change we can turn on kafka throttling on the job instance level.
Diffs (updated)
-----
samza-kafka/src/main/scala/org/apache/samza/config/KafkaConfig.scala 18225118ee5a3c70898274b28fe4ff0a11b55a12
samza-kafka/src/main/scala/org/apache/samza/config/RegExTopicGenerator.scala 78467bf1298653242e0b14d810e53a92a2ed139f
samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemConsumer.scala b37375360bcad8e3333089d95e8e3da63a1b9aab
samza-kafka/src/main/scala/org/apache/samza/util/KafkaUtil.scala a25ba620a58153b47059fbc11ae067f863e7c3cd
samza-kafka/src/test/scala/org/apache/samza/config/TestKafkaConfig.scala c4a83f60cf049e6fd65f940b69674895d07ab871
samza-kafka/src/test/scala/org/apache/samza/system/kafka/TestKafkaSystemConsumer.scala ece0359d1a31e65113f4832dd3baddb788cecef4
Diff: https://reviews.apache.org/r/50451/diff/
Testing
-------
gradle build & tests
Thanks,
Xinyu Liu
Re: Review Request 50451: SAMZA-981: Set consistent Kafka clientId
for a job instance
Posted by Xinyu Liu <xi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50451/
-----------------------------------------------------------
(Updated July 26, 2016, 11:44 p.m.)
Review request for samza, Navina Ramesh and Yi Pan (Data Infrastructure).
Changes
-------
Update based on Navina's feedback.
Repository: samza
Description
-------
Remove the currentTimeMills and counter in the ClientId creation so the clientId will be the same for each job instance. With this change we can turn on kafka throttling on the job instance level.
Diffs (updated)
-----
samza-kafka/src/main/scala/org/apache/samza/config/KafkaConfig.scala 18225118ee5a3c70898274b28fe4ff0a11b55a12
samza-kafka/src/main/scala/org/apache/samza/config/RegExTopicGenerator.scala 78467bf1298653242e0b14d810e53a92a2ed139f
samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemConsumer.scala b37375360bcad8e3333089d95e8e3da63a1b9aab
samza-kafka/src/main/scala/org/apache/samza/util/KafkaUtil.scala a25ba620a58153b47059fbc11ae067f863e7c3cd
samza-kafka/src/test/scala/org/apache/samza/config/TestKafkaConfig.scala c4a83f60cf049e6fd65f940b69674895d07ab871
samza-kafka/src/test/scala/org/apache/samza/system/kafka/TestKafkaSystemConsumer.scala ece0359d1a31e65113f4832dd3baddb788cecef4
Diff: https://reviews.apache.org/r/50451/diff/
Testing
-------
gradle build & tests
Thanks,
Xinyu Liu
Re: Review Request 50451: SAMZA-981: Set consistent Kafka clientId
for a job instance
Posted by Navina Ramesh <nr...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50451/#review143591
-----------------------------------------------------------
Fix it, then Ship it!
lgtm!
samza-kafka/src/test/scala/org/apache/samza/config/TestKafkaConfig.scala (line 168)
<https://reviews.apache.org/r/50451/#comment209417>
nit: may be you can use a constance for "TestClientId"??
- Navina Ramesh
On July 26, 2016, 6:15 p.m., Xinyu Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50451/
> -----------------------------------------------------------
>
> (Updated July 26, 2016, 6:15 p.m.)
>
>
> Review request for samza, Navina Ramesh and Yi Pan (Data Infrastructure).
>
>
> Repository: samza
>
>
> Description
> -------
>
> Remove the currentTimeMills and counter in the ClientId creation so the clientId will be the same for each job instance. With this change we can turn on kafka throttling on the job instance level.
>
>
> Diffs
> -----
>
> samza-kafka/src/main/scala/org/apache/samza/config/KafkaConfig.scala 18225118ee5a3c70898274b28fe4ff0a11b55a12
> samza-kafka/src/main/scala/org/apache/samza/config/RegExTopicGenerator.scala 78467bf1298653242e0b14d810e53a92a2ed139f
> samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemConsumer.scala b37375360bcad8e3333089d95e8e3da63a1b9aab
> samza-kafka/src/main/scala/org/apache/samza/util/KafkaUtil.scala a25ba620a58153b47059fbc11ae067f863e7c3cd
> samza-kafka/src/test/scala/org/apache/samza/config/TestKafkaConfig.scala c4a83f60cf049e6fd65f940b69674895d07ab871
> samza-kafka/src/test/scala/org/apache/samza/system/kafka/TestKafkaSystemConsumer.scala ece0359d1a31e65113f4832dd3baddb788cecef4
>
> Diff: https://reviews.apache.org/r/50451/diff/
>
>
> Testing
> -------
>
> gradle build & tests
>
>
> Thanks,
>
> Xinyu Liu
>
>
Re: Review Request 50451: SAMZA-981: Set consistent Kafka clientId
for a job instance
Posted by Xinyu Liu <xi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50451/
-----------------------------------------------------------
(Updated July 26, 2016, 6:15 p.m.)
Review request for samza, Navina Ramesh and Yi Pan (Data Infrastructure).
Repository: samza
Description (updated)
-------
Remove the currentTimeMills and counter in the ClientId creation so the clientId will be the same for each job instance. With this change we can turn on kafka throttling on the job instance level.
Diffs
-----
samza-kafka/src/main/scala/org/apache/samza/config/KafkaConfig.scala 18225118ee5a3c70898274b28fe4ff0a11b55a12
samza-kafka/src/main/scala/org/apache/samza/config/RegExTopicGenerator.scala 78467bf1298653242e0b14d810e53a92a2ed139f
samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemConsumer.scala b37375360bcad8e3333089d95e8e3da63a1b9aab
samza-kafka/src/main/scala/org/apache/samza/util/KafkaUtil.scala a25ba620a58153b47059fbc11ae067f863e7c3cd
samza-kafka/src/test/scala/org/apache/samza/config/TestKafkaConfig.scala c4a83f60cf049e6fd65f940b69674895d07ab871
samza-kafka/src/test/scala/org/apache/samza/system/kafka/TestKafkaSystemConsumer.scala ece0359d1a31e65113f4832dd3baddb788cecef4
Diff: https://reviews.apache.org/r/50451/diff/
Testing
-------
gradle build & tests
Thanks,
Xinyu Liu