Re: Review Request 17537: Patch for KAFKA-1028

2014-03-17 Thread Neha Narkhede
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17537/#review37419 --- Ship it! Ship It! - Neha Narkhede On March 17, 2014, 2:39 p.m.,

Re: Review Request 17537: Patch for KAFKA-1028

2014-03-17 Thread Andrew Olson
> On March 17, 2014, 4:30 p.m., Jay Kreps wrote: > > core/src/main/scala/kafka/server/KafkaConfig.scala, line 256 > > > > > > Are these config changes due to needing a rebase or something? They > > don't seem relate

Re: Review Request 17537: Patch for KAFKA-1028

2014-03-17 Thread Jay Kreps
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17537/#review37386 --- core/src/main/scala/kafka/server/KafkaConfig.scala

Re: Review Request 17537: Patch for KAFKA-1028

2014-03-17 Thread Andrew Olson
> On March 14, 2014, 8:13 p.m., Jay Kreps wrote: > > This is great! > > > > I don't think we should add a special purpose method that hard codes one > > property in AdminUtils, I think the helper code is actually there in > > LogConfig. > > > > I see Neha's point about adding a higher-level c

Re: Review Request 17537: Patch for KAFKA-1028

2014-03-17 Thread Andrew Olson
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17537/ --- (Updated March 17, 2014, 2:39 p.m.) Review request for kafka. Bugs: KAFKA-102

Re: Review Request 17537: Patch for KAFKA-1028

2014-03-15 Thread Neha Narkhede
> On March 14, 2014, 8:13 p.m., Jay Kreps wrote: > > This is great! > > > > I don't think we should add a special purpose method that hard codes one > > property in AdminUtils, I think the helper code is actually there in > > LogConfig. > > > > I see Neha's point about adding a higher-level c

Re: Review Request 17537: Patch for KAFKA-1028

2014-03-14 Thread Neha Narkhede
> On March 14, 2014, 8:13 p.m., Jay Kreps wrote: > > This is great! > > > > I don't think we should add a special purpose method that hard codes one > > property in AdminUtils, I think the helper code is actually there in > > LogConfig. > > > > I see Neha's point about adding a higher-level c

Re: Review Request 17537: Patch for KAFKA-1028

2014-03-14 Thread Andrew Olson
> On March 14, 2014, 8:13 p.m., Jay Kreps wrote: > > This is great! > > > > I don't think we should add a special purpose method that hard codes one > > property in AdminUtils, I think the helper code is actually there in > > LogConfig. > > > > I see Neha's point about adding a higher-level c

Re: Review Request 17537: Patch for KAFKA-1028

2014-03-14 Thread Jay Kreps
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17537/#review37265 --- Ship it! This is great! I don't think we should add a special purp

Re: Review Request 17537: Patch for KAFKA-1028

2014-03-03 Thread Andrew Olson
> On Feb. 4, 2014, 11:21 p.m., Neha Narkhede wrote: > > core/src/main/scala/kafka/server/ReplicaFetcherThread.scala, line 86 > > > > > > Reading from zookeeper is unnecessary here, since the broker has a > > mechanism

Re: Review Request 17537: Patch for KAFKA-1028

2014-03-03 Thread Andrew Olson
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17537/ --- (Updated March 4, 2014, 12:48 a.m.) Review request for kafka. Bugs: KAFKA-102

Re: Review Request 17537: Patch for KAFKA-1028

2014-03-03 Thread Guozhang Wang
> On March 3, 2014, 7:03 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/admin/AdminUtils.scala, line 356 > > > > > > "unclean leader" is a bit confusing, how about > > uncleanLeaderElectionEnabled? > > Andr

Re: Review Request 17537: Patch for KAFKA-1028

2014-03-03 Thread Andrew Olson
> On March 3, 2014, 7:03 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/server/ReplicaFetcherThread.scala, line 86 > > > > > > I am not sure we need to check the unclean election flag here. If we > > can keep

Re: Review Request 17537: Patch for KAFKA-1028

2014-03-03 Thread Andrew Olson
> On March 3, 2014, 7:03 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala, line 64 > > > > > > Not sure throwing an exception is the right behavior here? If unclean > >

Re: Review Request 17537: Patch for KAFKA-1028

2014-03-03 Thread Andrew Olson
> On March 3, 2014, 7:03 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/admin/AdminUtils.scala, line 356 > > > > > > "unclean leader" is a bit confusing, how about > > uncleanLeaderElectionEnabled? Sounds g

Re: Review Request 17537: Patch for KAFKA-1028

2014-03-03 Thread Guozhang Wang
> On Feb. 4, 2014, 11:21 p.m., Neha Narkhede wrote: > > core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala, line 64 > > > > > > the config object should already have the per topic preference for > > unc

Re: Review Request 17537: Patch for KAFKA-1028

2014-03-03 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17537/#review35999 --- core/src/main/scala/kafka/admin/AdminUtils.scala

Re: Review Request 17537: Patch for KAFKA-1028

2014-02-07 Thread Andrew Olson
> On Feb. 4, 2014, 11:21 p.m., Neha Narkhede wrote: > > core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala, line 64 > > > > > > the config object should already have the per topic preference for > > unc

Re: Review Request 17537: Patch for KAFKA-1028

2014-02-07 Thread Neha Narkhede
> On Feb. 4, 2014, 11:21 p.m., Neha Narkhede wrote: > > core/src/main/scala/kafka/server/ReplicaFetcherThread.scala, line 86 > > > > > > Reading from zookeeper is unnecessary here, since the broker has a > > mechanism

Re: Review Request 17537: Patch for KAFKA-1028

2014-02-05 Thread Andrew Olson
> On Feb. 4, 2014, 11:21 p.m., Neha Narkhede wrote: > > core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala, line 64 > > > > > > the config object should already have the per topic preference for > > unc

Re: Review Request 17537: Patch for KAFKA-1028

2014-02-04 Thread Neha Narkhede
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17537/#review33658 --- core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala

Re: Review Request 17537: Patch for KAFKA-1028

2014-01-30 Thread Andrew Olson
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17537/ --- (Updated Jan. 30, 2014, 7:45 p.m.) Review request for kafka. Bugs: KAFKA-1028

Re: Review Request 17537: Patch for KAFKA-1028

2014-01-30 Thread Andrew Olson
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17537/ --- (Updated Jan. 30, 2014, 7:42 p.m.) Review request for kafka. Bugs: KAFKA-1028

Review Request 17537: Patch for KAFKA-1028

2014-01-30 Thread Andrew Olson
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17537/ --- Review request for kafka. Bugs: KAFKA-1028 https://issues.apache.org/jira/b