Re: Review Request 20471: Patch for KAFKA-1398

2014-04-18 Thread Joel Koshy
> On April 18, 2014, 3:22 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/TopicConfigManager.scala, lines 123-124 > > > > > > This is not needed. In ZK, new sequential nodes always get a higher id > > whethe

Re: Review Request 20471: Patch for KAFKA-1398

2014-04-18 Thread Joel Koshy
> On April 18, 2014, 6:12 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/TopicConfigManager.scala, lines 87-88 > > > > > > Actually that's not the right example - it would work fine since you > > only wan

Re: Review Request 20471: Patch for KAFKA-1398

2014-04-18 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20471/#review40792 --- core/src/main/scala/kafka/server/TopicConfigManager.scala

Re: Review Request 20471: Patch for KAFKA-1398

2014-04-18 Thread Jay Kreps
> On April 18, 2014, 6:01 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/TopicConfigManager.scala, lines 87-88 > > > > > > Say you override the retention to x hours, realize you made a mistake > > and the

Re: Review Request 20471: Patch for KAFKA-1398

2014-04-18 Thread Jun Rao
> On April 18, 2014, 3:22 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/TopicConfigManager.scala, lines 123-124 > > > > > > This is not needed. In ZK, new sequential nodes always get a higher id > > whethe

Re: Review Request 20471: Patch for KAFKA-1398

2014-04-18 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20471/#review40786 --- core/src/main/scala/kafka/server/TopicConfigManager.scala

Re: Review Request 20471: Patch for KAFKA-1398

2014-04-18 Thread Jay Kreps
> On April 18, 2014, 3:22 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/TopicConfigManager.scala, lines 87-88 > > > > > > The child list returned by ZK doesn't guarantee any ordering. We will > > need to so

Re: Review Request 20471: Patch for KAFKA-1398

2014-04-18 Thread Jay Kreps
> On April 18, 2014, 5:25 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/TopicConfigManager.scala, lines 121-134 > > > > > > You mean that the purge is only triggered by a broker bounce or a new > > conf

Re: Review Request 20471: Patch for KAFKA-1398

2014-04-18 Thread Jun Rao
> On April 18, 2014, 3:22 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/TopicConfigManager.scala, lines 97-98 > > > > > > In the corner case, it seems that it's still possible by the time that > > we try to

Re: Review Request 20471: Patch for KAFKA-1398

2014-04-18 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20471/#review40780 --- core/src/main/scala/kafka/server/TopicConfigManager.scala

Re: Review Request 20471: Patch for KAFKA-1398

2014-04-18 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20471/#review40772 --- Could you add some comments at the beginning of TopicConfigManager t

Re: Review Request 20471: Patch for KAFKA-1398

2014-04-17 Thread Jay Kreps
> On April 18, 2014, 1:17 a.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/TopicConfigManager.scala, line 62 > > > > > > Since this is not currently exposed as a config should we just remove > > the paramet

Re: Review Request 20471: Patch for KAFKA-1398

2014-04-17 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20471/#review40726 --- Ship it! Looks good except for the minor issues below. core/src/m

Review Request 20471: Patch for KAFKA-1398

2014-04-17 Thread Jay Kreps
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20471/ --- Review request for kafka. Bugs: KAFKA-1398 https://issues.apache.org/jira/b