gharris1727 commented on code in PR #16479:
URL: https://github.com/apache/kafka/pull/16479#discussion_r1729520016
##########
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java:
##########
@@ -253,6 +253,7 @@ public class KafkaProducer<K, V> implements Producer<K, V> {
private final RecordAccumulator accumulator;
private final Sender sender;
private final Thread ioThread;
+ private Throwable uncaughtExceptionOfIoThread;
Review Comment:
> However, i seems that Sender's constructor should be modified. I don't
know whether we can do that?
I don't know what you mean. I don't think that the Sender constructor would
need to be changed. Maybe a new method on the sender would be required to check
it's state. The Sender is an internal class, so we don't have to worry about
compatibility here.
> Change org.apache.kafka.clients.producer.internals.Sender#run from catch
Exception to Throwable. However, I don't know why Sender also just log the
error rather than throw it? Is there any concerns if we throw the exception to
user?
I think the idea is that the background thread should never stop unless it
has been requested to stop, and that exceptions thrown from somewhere in that
thread should just skip to the next iteration, hoping that whatever error
occurred was transient and the producer can continue to operate.
At the same time, I have also heard that "Exceptions" are meant to be
handled by user code, and "Errors" are not, and broadening those catch clauses
to Throwable would mean we would catch things like OutOfMemoryErrors,
LinkageErrors, etc that are very severe, and would be bad to retry in a tight
loop.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
