> On July 3, 2015, 1:36 a.m., Guozhang Wang wrote: > > Thanks for the patch. I have a few thoughts regarding the names of the > > metrics, since in the producer other causes can also result in dropped > > messages (i.e. rejected before it enteres the producer buffer), such as > > message-size-too-large, serialization-failed, etc. In the old producer > > since we only have one cause we named that to droppedMessageRate. So I > > think we could either: > > > > 1. record dropped messages for any KafkaExceptions, but not limited to > > BufferExhaustedException. > > 2. have a separate metric for buffer-exhausted with a different name. > > > > I prefer the first option since I feel in practice people just want to > > distinguish between the case that messages failed to get into the producer > > from the case the messages gets failed to send to the broker. > > Dong Lin wrote: > Thanks for your comments. If I understand it right, we already have a > sensor named "errors" which records the message dropped for any > KafkaExceptions. Therefore no work is needed option 1. > > I think there is probably need in opertaion to track the number of > messages dropped due to BufferExhaustedException. Because 1) this is a common > exception may happen un-noticed if block_on_buffer_full=false and > asynchronous producer is used; and 2) for backward compatibility with old > producer. I will ask Joel if he has more detail on the motivation. > > Guozhang Wang wrote: > There are two cenarios that a message does not successfully reach the > broker: > > 1. The message gets rejected by the producer immediately before being > added to the producer's buffer for sending. This error is thrown as > non-ApiException KafkaException. > 2. The message was sent to the broker from the producer's send buffer but > got rejected (and later retries exhausted). This error is thrown as > ApiException. > > Currently both of them are recorded as record-error-rate, while in the > old producer, we record DroppedRecord for the first scenario, which only > includes BufferFullException. > > So I was proposing if we want back-ward compatibility we could record the > first scenario, which include BufferExhausted but also some other exceptions > in a separate metric. Does that sound reasonable? > > Dong Lin wrote: > Yeah I understand your suggestion. But could you please explain why > having a sensor for all non-ApiException KafkaException is better than > ApiException KafkaException only? > > I think it wouldn't be exactly backward compatible if you include other > exceptions, such as ClassCastException, in this sensor. It could cause > problem to users if they are depending on this sensor to measure how many > data are dropped in asynchronous call due to BufferFullException. > > What do you think? > > Dong Lin wrote: > oops. I mean SerializationException, not ClassCastException, in the > comment above. > > BTW, the addition of this sensor is motivated when we prepare the release > note of new producer. I think if this sensor is not important, we don't need > to add it. Otherwise, it is easier to explain to user with its original > definition. > > Guozhang Wang wrote: > I think you meant to say "why having a sensor for all non-ApiException > KafkaException is better than BufferExhaustedException only", right? > > I do not preferring to having a metric for all non-ApiExceptions, as I > said in the first comment, we could do this (as option 1) or use a different > name as "record-dropped-rate" since it is too general as referring to any > non-ApiException that cause it to drop messages, for example > "record-dropped-due-to-memory-exhausted-rate" for BufferExhaustedException > only if users just want that (as option 2).
Yeah, that is my typo.. Thanks for the comment. Yes the "record-dropped-rate" is confusing. Could you please see if the updated patch addresses the issue? I used record-buffer-exhausted-rate since record-dropped-due-to-memory-exhausted-rate is probably too long. - Dong ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36034/#review90303 ----------------------------------------------------------- On July 6, 2015, 9:54 p.m., Dong Lin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36034/ > ----------------------------------------------------------- > > (Updated July 6, 2015, 9:54 p.m.) > > > Review request for kafka and Joel Koshy. > > > Bugs: KAFKA-2306 > https://issues.apache.org/jira/browse/KAFKA-2306 > > > Repository: kafka > > > Description > ------- > > KAFKA-2306; New producer should emit metrics for buffer exhaustion > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java > 5671a3fbeea8cb9a9ffeeb41aa1b132b92c0cae8 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java > 0baf16e55046a2f49f6431e01d52c323c95eddf0 > > Diff: https://reviews.apache.org/r/36034/diff/ > > > Testing > ------- > > > Thanks, > > Dong Lin > >
