> 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?
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 ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36034/#review90303 ----------------------------------------------------------- On July 3, 2015, 1:56 a.m., Dong Lin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36034/ > ----------------------------------------------------------- > > (Updated July 3, 2015, 1:56 a.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 > >
