Re: Review Request 25995: Patch for KAFKA-1650

> On Feb. 28, 2015, 4:43 p.m., Jun Rao wrote: > > Sorry for the late review. A few more comments below. Thanks a lot for the review, Jun. We actually have a new design for mirror maker based on the flush() call of producer (KAFKA-1865). The design is updated in KIP-3 and it is in voting proces

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/#review74677 --- Sorry for the late review. A few more comments below. core/src/mai

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/#review65993 --- Ship it! Ship It! - Joel Koshy On Dec. 24, 2014, 12:44 a.m., Jia

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/ --- (Updated Dec. 24, 2014, 12:44 a.m.) Review request for kafka. Bugs: KAFKA-165

Re: Review Request 25995: Patch for KAFKA-1650

> On Dec. 23, 2014, 11:53 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, line 58 > > > > > > I think this might be confusing. i.e., if we have an explicit argument > > for no data loss

Re: Review Request 25995: Patch for KAFKA-1650

> On Dec. 23, 2014, 11:53 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, line 58 > > > > > > I think this might be confusing. i.e., if we have an explicit argument > > for no data loss

Re: Review Request 25995: Patch for KAFKA-1650

> On Dec. 23, 2014, 11:53 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, line 58 > > > > > > I think this might be confusing. i.e., if we have an explicit argument > > for no data loss

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/#review65981 --- Ship it! Minor observation below. Also, mirror maker has become a

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/ --- (Updated Dec. 23, 2014, 3:04 p.m.) Review request for kafka. Bugs: KAFKA-1650

Re: Review Request 25995: Patch for KAFKA-1650

> On Dec. 23, 2014, 8:34 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, line 90 > > > > > > Use valuesIterator instead of values - the reason is that values > > materializes, but the it

Re: Review Request 25995: Patch for KAFKA-1650

> On Dec. 22, 2014, 1:34 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, line 85 > > > > > > NumUnackedOffsets is a "weird" metric especially with the presence of > > NumUnackedMessages.

Re: Review Request 25995: Patch for KAFKA-1650

> On Dec. 23, 2014, 8:34 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, line 90 > > > > > > Use valuesIterator instead of values - the reason is that values > > materializes, but the it

Re: Review Request 25995: Patch for KAFKA-1650

> On Dec. 22, 2014, 1:34 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala, line > > 394 > > > > > > if isAutoCommit is true then we will not retry anyway. I think this

Re: Review Request 25995: Patch for KAFKA-1650

> On Dec. 18, 2014, 10:42 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, line 614 > > > > > > Should this be fatal? i.e., fatal is normally used before exiting > > (abnormally). WARN w

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/#review65874 --- Looks great. Just a few more minor comments + a question that you ma

Re: Review Request 25995: Patch for KAFKA-1650

> On Dec. 18, 2014, 10:42 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, line 614 > > > > > > Should this be fatal? i.e., fatal is normally used before exiting > > (abnormally). WARN w

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/ --- (Updated Dec. 23, 2014, 3:07 a.m.) Review request for kafka. Bugs: KAFKA-1650

Re: Review Request 25995: Patch for KAFKA-1650

> On Dec. 18, 2014, 10:42 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, line 614 > > > > > > Should this be fatal? i.e., fatal is normally used before exiting > > (abnormally). WARN w

Re: Review Request 25995: Patch for KAFKA-1650

> On Dec. 17, 2014, 1:17 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, line 489 > > > > > > Why not use KafkaScheduler for the offset commit task? > > Jiangjie Qin wrote: > Haven'

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/#review65759 --- core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala

Re: Review Request 25995: Patch for KAFKA-1650

> On Dec. 17, 2014, 1:17 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, line 489 > > > > > > Why not use KafkaScheduler for the offset commit task? > > Jiangjie Qin wrote: > Haven'

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/ --- (Updated Dec. 19, 2014, 7:41 a.m.) Review request for kafka. Bugs: KAFKA-1650

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/ --- (Updated Dec. 19, 2014, 6:53 a.m.) Review request for kafka. Bugs: KAFKA-1650

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/ --- (Updated Dec. 19, 2014, 6:17 a.m.) Review request for kafka. Bugs: KAFKA-1650

Re: Review Request 25995: Patch for KAFKA-1650

> On Dec. 18, 2014, 10:42 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, line 614 > > > > > > Should this be fatal? i.e., fatal is normally used before exiting > > (abnormally). WARN w

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/ --- (Updated Dec. 19, 2014, 2:48 a.m.) Review request for kafka. Bugs: KAFKA-1650

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/#review65477 --- core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala

Re: Review Request 25995: Patch for KAFKA-1650

> On Dec. 17, 2014, 1:17 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala, line > > 338 > > > > > > Can we make this a "reliable" commit - i.e., with retries up to the

Re: Review Request 25995: Patch for KAFKA-1650

> On Dec. 17, 2014, 1:17 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala, line > > 338 > > > > > > Can we make this a "reliable" commit - i.e., with retries up to the

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/ --- (Updated Dec. 17, 2014, 8:29 p.m.) Review request for kafka. Bugs: KAFKA-1650

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/#review65306 --- core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/ --- (Updated Dec. 16, 2014, 4:03 p.m.) Review request for kafka. Bugs: KAFKA-1650

Re: Review Request 25995: Patch for KAFKA-1650

> On Dec. 10, 2014, 5:09 a.m., Jun Rao wrote: > > Haven't looked at your new patch yet. It seems that the following tests > > start to fail after your commit. They seem to pass if run individually, but > > always fail if you run all unit tests. Could you take a look and at least > > understand

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/#review64505 --- Haven't looked at your new patch yet. It seems that the following te

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/ --- (Updated Dec. 8, 2014, 9:36 a.m.) Review request for kafka. Bugs: KAFKA-1650

Re: Review Request 25995: Patch for KAFKA-1650

> On Dec. 6, 2014, 5:17 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, lines 529-530 > > > > > > Does that imply there is always an offset? Is that always true? > > > > I don't qu

Re: Review Request 25995: Patch for KAFKA-1650

> On Dec. 6, 2014, 5:17 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, lines 529-530 > > > > > > Does that imply there is always an offset? Is that always true? > > > > I don't qu

Re: Review Request 25995: Patch for KAFKA-1650

> On Dec. 6, 2014, 5:17 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, lines 529-530 > > > > > > Does that imply there is always an offset? Is that always true? > > > > I don't qu

Re: Review Request 25995: Patch for KAFKA-1650

> On Dec. 6, 2014, 5:17 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, lines 529-530 > > > > > > Does that imply there is always an offset? Is that always true? > > > > I don't qu

Re: Review Request 25995: Patch for KAFKA-1650

> On Dec. 6, 2014, 5:17 p.m., Jun Rao wrote: > > Thanks for the patch. Some comments below. Thank you very much for the review. > On Dec. 6, 2014, 5:17 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, lines 529-530 > >

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/ --- (Updated Dec. 7, 2014, 2:59 a.m.) Review request for kafka. Bugs: KAFKA-1650

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/#review64152 --- Thanks for the patch. Some comments below. core/src/main/scala/kaf

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/ --- (Updated Dec. 4, 2014, 7:59 p.m.) Review request for kafka. Bugs: KAFKA-1650

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/#review63878 --- Ship it! Ship It! - Guozhang Wang On Dec. 4, 2014, 3:02 a.m., Ji

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/#review63703 --- core/src/main/scala/kafka/tools/MirrorMaker.scala

Re: Review Request 25995: Patch for KAFKA-1650

> On Dec. 3, 2014, 6:57 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, line 613 > > > > > > I think there may be a race condition here, for example consider this > > sequence: > >

Re: Review Request 25995: Patch for KAFKA-1650

> On Dec. 3, 2014, 6:57 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, line 613 > > > > > > I think there may be a race condition here, for example consider this > > sequence: > >

Re: Review Request 25995: Patch for KAFKA-1650

> On Dec. 3, 2014, 6:57 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, line 613 > > > > > > I think there may be a race condition here, for example consider this > > sequence: > >

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/ --- (Updated Dec. 4, 2014, 3:02 a.m.) Review request for kafka. Bugs: KAFKA-1650

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/ --- (Updated Dec. 3, 2014, 11:02 p.m.) Review request for kafka. Bugs: KAFKA-1650

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/ --- (Updated Nov. 24, 2014, 4:15 p.m.) Review request for kafka. Bugs: KAFKA-1650

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/ --- (Updated Nov. 20, 2014, 8 p.m.) Review request for kafka. Bugs: KAFKA-1650 an

Re: Review Request 25995: Patch for KAFKA-1650

> On Nov. 17, 2014, 9:48 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/tools/MirrorMaker.scala, line 338 > > > > > > Is it possible that the current chunk has been consumed completely and > > the fetcher th

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/ --- (Updated Nov. 18, 2014, 2:44 a.m.) Review request for kafka. Bugs: KAFKA-1650

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/#review61528 --- core/src/main/scala/kafka/tools/MirrorMaker.scala

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/ --- (Updated Nov. 12, 2014, 5:51 p.m.) Review request for kafka. Summary (updated

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/#review60659 --- core/src/main/scala/kafka/tools/MirrorMaker.scala

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/#review55612 --- core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/ --- (Updated Oct. 6, 2014, 5:20 p.m.) Review request for kafka. Bugs: KAFKA-1650

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/ --- (Updated Oct. 6, 2014, 5:17 p.m.) Review request for kafka. Bugs: KAFKA-1650

Re: Review Request 25995: Patch for KAFKA-1650

> On Sept. 25, 2014, 11:48 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala, line > > 299 > > > > > > Do we need to do this check every time in the loop? Maybe we can

Re: Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/#review54620 --- core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala

Review Request 25995: Patch for KAFKA-1650

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25995/ --- Review request for kafka. Bugs: KAFKA-1650 https://issues.apache.org/jira/b