----------------------------------------------------------- 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 > >
