----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52403/#review150951 -----------------------------------------------------------
samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemProducer.scala (lines 60 - 61) <https://reviews.apache.org/r/52403/#comment219055> To me, this reads as: "We don't queue up more send requests from the Samza thread since we want to store the exception in case of ultimate send failure in the I/O thread.". But that doesn't make sense since we only stop queueing messages _after_ we see this value set. I think this is meant to be the other way around? I.e, "exceptionInCallback: stores the exception in case of any "ultimate" send failure (ie. failure after exhausting max_retries in Kafka producer) in the I/O thread, so that we do not continue to queue up more send requests from the Samza thread." The second sentence could also be: This helps the Samza thread know about I/O thread failures. (since w/o this field the Samza thread does not even know of a I/O failure, so the source is a moot point). - Prateek Maheshwari On Sept. 29, 2016, 2:23 p.m., Xinyu Liu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52403/ > ----------------------------------------------------------- > > (Updated Sept. 29, 2016, 2:23 p.m.) > > > Review request for samza, Navina Ramesh and Jagadish Venkatraman. > > > Repository: samza > > > Description > ------- > > Current the error log happens after produce close and reset the exception in > later callbacks, which caused the trouble shooting to be harder in cases of > multithreading. We should log error before closing and keep atomic reference > of the initial exception. > > > Diffs > ----- > > > samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemProducer.scala > 5ff6d3caf54ed148aa40c7c752c587e556a4f34a > > Diff: https://reviews.apache.org/r/52403/diff/ > > > Testing > ------- > > Tested in jobs deployed in Yarn cluster. > > > Thanks, > > Xinyu Liu > >
