Re: Review Request 36548: Patch for KAFKA-2336

> On Aug. 11, 2015, 10:08 p.m., Gwen Shapira wrote: > > Ship It! > > Gwen Shapira wrote: > Jiangjie, I commited despite your concerns since this patch fixes a huge > potential issue. > > If you have an idea for an improved fix, we can tackle this in a follow > up. Thanks Gwen. I

Re: Review Request 36548: Patch for KAFKA-2336

> On Aug. 11, 2015, 10:08 p.m., Gwen Shapira wrote: > > Ship It! Jiangjie, I commited despite your concerns since this patch fixes a huge potential issue. If you have an idea for an improved fix, we can tackle this in a follow up. - Gwen

Re: Review Request 36548: Patch for KAFKA-2336

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36548/#review95012 --- Ship it! Ship It! - Gwen Shapira On Aug. 11, 2015, 3:37 p.m., Gr

Re: Review Request 36548: Patch for KAFKA-2336

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36548/ --- (Updated Aug. 11, 2015, 3:37 p.m.) Review request for kafka. Bugs: KAFKA-2336

Re: Review Request 36548: Patch for KAFKA-2336

> On July 17, 2015, 4:26 a.m., Jiangjie Qin wrote: > > Looks good to me. > > Jiangjie Qin wrote: > Actually do we need to talk to Zookeeper every time? Can we read the data > from topic metadata cache directly? > > Gwen Shapira wrote: > Good point, Jiangjie - looks like partitionFor is

Re: Review Request 36548: Patch for KAFKA-2336

> On July 17, 2015, 4:26 a.m., Jiangjie Qin wrote: > > Looks good to me. > > Jiangjie Qin wrote: > Actually do we need to talk to Zookeeper every time? Can we read the data > from topic metadata cache directly? > > Gwen Shapira wrote: > Good point, Jiangjie - looks like partitionFor is

Re: Review Request 36548: Patch for KAFKA-2336

> On July 21, 2015, 2:43 p.m., Ismael Juma wrote: > > core/src/main/scala/kafka/server/OffsetManager.scala, line 454 > > > > > > Is `topicData` guaranteed to have a key for `topic`? If not, it's > > better to do `to

Re: Review Request 36548: Patch for KAFKA-2336

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36548/#review92406 --- core/src/main/scala/kafka/server/OffsetManager.scala (line 447)

Re: Review Request 36548: Patch for KAFKA-2336

> On July 17, 2015, 4:26 a.m., Jiangjie Qin wrote: > > Looks good to me. > > Jiangjie Qin wrote: > Actually do we need to talk to Zookeeper every time? Can we read the data > from topic metadata cache directly? > > Gwen Shapira wrote: > Good point, Jiangjie - looks like partitionFor is

Re: Review Request 36548: Patch for KAFKA-2336

> On July 17, 2015, 4:26 a.m., Jiangjie Qin wrote: > > Looks good to me. > > Jiangjie Qin wrote: > Actually do we need to talk to Zookeeper every time? Can we read the data > from topic metadata cache directly? > > Gwen Shapira wrote: > Good point, Jiangjie - looks like partitionFor is

Re: Review Request 36548: Patch for KAFKA-2336

> On July 17, 2015, 4:26 a.m., Jiangjie Qin wrote: > > Looks good to me. > > Jiangjie Qin wrote: > Actually do we need to talk to Zookeeper every time? Can we read the data > from topic metadata cache directly? > > Gwen Shapira wrote: > Good point, Jiangjie - looks like partitionFor is

Re: Review Request 36548: Patch for KAFKA-2336

> On July 17, 2015, 4:26 a.m., Jiangjie Qin wrote: > > Looks good to me. > > Jiangjie Qin wrote: > Actually do we need to talk to Zookeeper every time? Can we read the data > from topic metadata cache directly? > > Gwen Shapira wrote: > Good point, Jiangjie - looks like partitionFor is

Re: Review Request 36548: Patch for KAFKA-2336

> On July 17, 2015, 4:26 a.m., Jiangjie Qin wrote: > > Looks good to me. > > Jiangjie Qin wrote: > Actually do we need to talk to Zookeeper every time? Can we read the data > from topic metadata cache directly? Good point, Jiangjie - looks like partitionFor is called on every ConsumerMeta

Re: Review Request 36548: Patch for KAFKA-2336

> On July 17, 2015, 4:26 a.m., Jiangjie Qin wrote: > > Looks good to me. Actually do we need to talk to Zookeeper every time? Can we read the data from topic metadata cache directly? - Jiangjie --- This is an automatically generated e-

Re: Review Request 36548: Patch for KAFKA-2336

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36548/#review92020 --- Ship it! Looks good to me. - Jiangjie Qin On July 16, 2015, 6:04

Re: Review Request 36548: Patch for KAFKA-2336

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36548/ --- (Updated July 16, 2015, 6:04 p.m.) Review request for kafka. Bugs: KAFKA-2336

Re: Review Request 36548: Patch for KAFKA-2336

> On July 16, 2015, 4:53 p.m., Edward Ribeiro wrote: > > core/src/main/scala/kafka/server/OffsetManager.scala, line 454 > > > > > > It looks like there's a mix of "if(" and "if (", with the latter being > > more use

Re: Review Request 36548: Patch for KAFKA-2336

> On July 16, 2015, 4:53 p.m., Edward Ribeiro wrote: > > core/src/main/scala/kafka/server/OffsetManager.scala, line 454 > > > > > > It looks like there's a mix of "if(" and "if (", with the latter being > > more use

Re: Review Request 36548: Patch for KAFKA-2336

> On July 16, 2015, 4:53 p.m., Edward Ribeiro wrote: > > core/src/main/scala/kafka/server/OffsetManager.scala, line 454 > > > > > > It looks like there's a mix of "if(" and "if (", with the latter being > > more use

Re: Review Request 36548: Patch for KAFKA-2336

> On July 16, 2015, 4:53 p.m., Edward Ribeiro wrote: > > core/src/main/scala/kafka/server/OffsetManager.scala, line 454 > > > > > > It looks like there's a mix of "if(" and "if (", with the latter being > > more use

Re: Review Request 36548: Patch for KAFKA-2336

> On July 16, 2015, 4:53 p.m., Edward Ribeiro wrote: > > core/src/main/scala/kafka/server/OffsetManager.scala, line 454 > > > > > > It looks like there's a mix of "if(" and "if (", with the latter being > > more use

Re: Review Request 36548: Patch for KAFKA-2336

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36548/#review91898 --- core/src/main/scala/kafka/server/OffsetManager.scala (line 453)

Review Request 36548: Patch for KAFKA-2336

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