Re: Review Request 52403: SAMZA-1028: Moving logline before closing kafka producer and making exception thrown AtomicReference

2016-10-18 Thread Prateek Maheshwari
> On Sept. 29, 2016, 2:30 p.m., Prateek Maheshwari wrote: > > samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemProducer.scala, > > line 220 > > > > > > If the intention is to allow user to ensure

Re: Review Request 52403: SAMZA-1028: Moving logline before closing kafka producer and making exception thrown AtomicReference

2016-09-29 Thread Prateek Maheshwari
--- 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/KafkaSys

Re: Review Request 52403: SAMZA-1028: Moving logline before closing kafka producer and making exception thrown AtomicReference

2016-09-29 Thread Xinyu Liu
> On Sept. 29, 2016, 9:31 p.m., Yi Pan (Data Infrastructure) wrote: > > samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemProducer.scala, > > line 59 > > > > > > nit: be consistent w/ the code: exce

Re: Review Request 52403: SAMZA-1028: Moving logline before closing kafka producer and making exception thrown AtomicReference

2016-09-29 Thread Yi Pan (Data Infrastructure)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52403/#review150948 --- Ship it! lgtm. Thanks! samza-kafka/src/main/scala/org/apache/

Re: Review Request 52403: SAMZA-1028: Moving logline before closing kafka producer and making exception thrown AtomicReference

2016-09-29 Thread Prateek Maheshwari
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52403/#review150947 --- samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSys

Re: Review Request 52403: SAMZA-1028: Moving logline before closing kafka producer and making exception thrown AtomicReference

2016-09-29 Thread Xinyu Liu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52403/ --- (Updated Sept. 29, 2016, 9:23 p.m.) Review request for samza, Navina Ramesh and

Re: Review Request 52403: SAMZA-1028: Moving logline before closing kafka producer and making exception thrown AtomicReference

2016-09-29 Thread Prateek Maheshwari
> On Sept. 29, 2016, 12:33 p.m., Prateek Maheshwari wrote: > > samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemProducer.scala, > > line 202 > > > > > > Wondering why we rethrow a previously saved

Re: Review Request 52403: SAMZA-1028: Moving logline before closing kafka producer and making exception thrown AtomicReference

2016-09-29 Thread Xinyu Liu
> On Sept. 29, 2016, 7:33 p.m., Prateek Maheshwari wrote: > > samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemProducer.scala, > > line 65 > > > > > > maybe firstCallbackException? > > Don't th

Re: Review Request 52403: SAMZA-1028: Moving logline before closing kafka producer and making exception thrown AtomicReference

2016-09-29 Thread Xinyu Liu
> On Sept. 29, 2016, 7:08 p.m., Chris Pettitt wrote: > > You're not using an atomic compare and set (CAS), so you don't need an > > atomic ref - a volatile would be sufficient. However, if the code can be > > run in a multi-threaded path, you would indeed want to use CAS to dequeue > > the exc

Re: Review Request 52403: SAMZA-1028: Moving logline before closing kafka producer and making exception thrown AtomicReference

2016-09-29 Thread Xinyu Liu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52403/ --- (Updated Sept. 29, 2016, 9 p.m.) Review request for samza, Navina Ramesh and Ja

Re: Review Request 52403: SAMZA-1028: Moving logline before closing kafka producer and making exception thrown AtomicReference

2016-09-29 Thread Prateek Maheshwari
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52403/#review150909 --- samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSys

Review Request 52403: SAMZA-1028: Moving logline before closing kafka producer and making exception thrown AtomicReference

2016-09-29 Thread Xinyu Liu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52403/ --- Review request for samza. Repository: samza Description --- Current the

Re: Review Request 52403: SAMZA-1028: Moving logline before closing kafka producer and making exception thrown AtomicReference

2016-09-29 Thread Chris Pettitt
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52403/#review150907 --- You're not using an atomic compare and set (CAS), so you don't nee