You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Edward Ribeiro <ed...@gmail.com> on 2015/09/03 00:31:00 UTC
Re: Review Request 36578: Patch for KAFKA-2338
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36578/
-----------------------------------------------------------
(Updated Sept. 2, 2015, 10:30 p.m.)
Review request for kafka.
Bugs: KAFKA-2338
https://issues.apache.org/jira/browse/KAFKA-2338
Repository: kafka
Description
-------
KAFKA-2338 Warn users if they change max.message.bytes that they also need to update broker and consumer settings
Diffs (updated)
-----
core/src/main/scala/kafka/admin/TopicCommand.scala f1405a5b2961bc826caa22507db8ba39ce1cf4d3
core/src/main/scala/kafka/server/AbstractFetcherThread.scala dca975ca1bf3e560b9d9817e7f2a511ef4296e70
Diff: https://reviews.apache.org/r/36578/diff/
Testing
-------
Thanks,
Edward Ribeiro
Re: Review Request 36578: Patch for KAFKA-2338
Posted by Edward Ribeiro <ed...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36578/#review98911
-----------------------------------------------------------
core/src/main/scala/kafka/admin/TopicCommand.scala (line 92)
<https://reviews.apache.org/r/36578/#comment155621>
Sure! Going to address this.
core/src/main/scala/kafka/server/AbstractFetcherThread.scala (lines 176 - 178)
<https://reviews.apache.org/r/36578/#comment155622>
Cool! Thanks for the heads up! Gonna get rid of this useless catch and modify accordingly.
- Edward Ribeiro
On Sept. 2, 2015, 10:30 p.m., Edward Ribeiro wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36578/
> -----------------------------------------------------------
>
> (Updated Sept. 2, 2015, 10:30 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-2338
> https://issues.apache.org/jira/browse/KAFKA-2338
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-2338 Warn users if they change max.message.bytes that they also need to update broker and consumer settings
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/admin/TopicCommand.scala f1405a5b2961bc826caa22507db8ba39ce1cf4d3
> core/src/main/scala/kafka/server/AbstractFetcherThread.scala dca975ca1bf3e560b9d9817e7f2a511ef4296e70
>
> Diff: https://reviews.apache.org/r/36578/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Edward Ribeiro
>
>
Re: Review Request 36578: Patch for KAFKA-2338
Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36578/#review97668
-----------------------------------------------------------
Thanks for the patch. A couple of comments below.
core/src/main/scala/kafka/admin/TopicCommand.scala (line 92)
<https://reviews.apache.org/r/36578/#comment153683>
Could we log what broker and consumer properties that the user need to change? Also, we should say what value the user needs to change to. This is a bit tricky with keys. So, the fetch size should be >= (max message size + key size + overhead).
core/src/main/scala/kafka/server/AbstractFetcherThread.scala (lines 176 - 178)
<https://reviews.apache.org/r/36578/#comment153684>
As Ewen pointed out earlier, this doesn't quite work. The broker never returns ErrorMapping.MessageSizeTooLargeCode. It's the responsibility of the consumer to check oversized messages. The way you check that is if messageSet.sizeInBytes > 0 && messageSet.validBytes <= 0, then there is an oversided message. Also, we should probably do the logging in ReplicaFetcherThread.processPartitionData() since for regular consumers, we already check oversized messages in ConsumerIterator.makeNext().
- Jun Rao
On Sept. 2, 2015, 10:30 p.m., Edward Ribeiro wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36578/
> -----------------------------------------------------------
>
> (Updated Sept. 2, 2015, 10:30 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-2338
> https://issues.apache.org/jira/browse/KAFKA-2338
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-2338 Warn users if they change max.message.bytes that they also need to update broker and consumer settings
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/admin/TopicCommand.scala f1405a5b2961bc826caa22507db8ba39ce1cf4d3
> core/src/main/scala/kafka/server/AbstractFetcherThread.scala dca975ca1bf3e560b9d9817e7f2a511ef4296e70
>
> Diff: https://reviews.apache.org/r/36578/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Edward Ribeiro
>
>