Re: Review Request 23702: Patch for KAFKA-1070

> On Jan. 12, 2015, 10:08 p.m., Neha Narkhede wrote: > > core/src/main/scala/kafka/server/BrokerMetadataCheckpoint.scala, line 55 > > > > > > Minor nit which I can address during checkin: According to our coding > > c

Re: Review Request 23702: Patch for KAFKA-1070

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23702/ --- (Updated Jan. 13, 2015, 2:30 a.m.) Review request for kafka. Bugs: KAFKA-1070

Re: Review Request 23702: Patch for KAFKA-1070

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23702/#review67715 --- Ship it! Ship It! core/src/main/scala/kafka/server/BrokerMetadata

Re: Review Request 23702: Patch for KAFKA-1070

> On Jan. 9, 2015, 11:39 p.m., Neha Narkhede wrote: > > core/src/main/scala/kafka/server/BrokerMetadataFileHandler.scala, line 48 > > > > > > Does java serialization for the Properties object allow you to read > > ju

Re: Review Request 23702: Patch for KAFKA-1070

> On Jan. 9, 2015, 11:39 p.m., Neha Narkhede wrote: > > core/src/main/scala/kafka/server/BrokerMetadataFileHandler.scala, line 48 > > > > > > Does java serialization for the Properties object allow you to read > > ju

Re: Review Request 23702: Patch for KAFKA-1070

> On Jan. 9, 2015, 11:39 p.m., Neha Narkhede wrote: > > core/src/main/scala/kafka/server/BrokerMetadataFileHandler.scala, line 48 > > > > > > Does java serialization for the Properties object allow you to read > > ju

Re: Review Request 23702: Patch for KAFKA-1070

> On Jan. 9, 2015, 11:39 p.m., Neha Narkhede wrote: > > core/src/main/scala/kafka/server/BrokerMetadataFileHandler.scala, line 48 > > > > > > Does java serialization for the Properties object allow you to read > > ju

Re: Review Request 23702: Patch for KAFKA-1070

> On Jan. 9, 2015, 11:39 p.m., Neha Narkhede wrote: > > core/src/main/scala/kafka/server/BrokerMetadataFileHandler.scala, line 30 > > > > > > I think the way you modelled this checkpoint file, which makes it > > differ

Re: Review Request 23702: Patch for KAFKA-1070

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23702/ --- (Updated Jan. 12, 2015, 6:46 p.m.) Review request for kafka. Bugs: KAFKA-1070

Re: Review Request 23702: Patch for KAFKA-1070

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23702/#review67551 --- core/src/main/scala/kafka/server/BrokerMetadataFileHandler.scala

Re: Review Request 23702: Patch for KAFKA-1070

> On Dec. 17, 2014, 5:21 a.m., Neha Narkhede wrote: > > core/src/main/scala/kafka/server/BrokerMetadataFileHandler.scala, line 46 > > > > > > If this crashes before syncing the data, it might lead to a corrupted > > me

Re: Review Request 23702: Patch for KAFKA-1070

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23702/ --- (Updated Jan. 2, 2015, 1:39 a.m.) Review request for kafka. Bugs: KAFKA-1070

Re: Review Request 23702: Patch for KAFKA-1070

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23702/#review65293 --- core/src/main/scala/kafka/common/GenerateBrokerIdException.scala

Re: Review Request 23702: Patch for KAFKA-1070

> On Dec. 17, 2014, 4:58 a.m., Neha Narkhede wrote: > > Overall, this looks good. How about adding some unit tests before checking > > it in? Please ignore this. I was looking at only part of the patch. - Neha --- This is an automatica

Re: Review Request 23702: Patch for KAFKA-1070

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23702/#review65289 --- Overall, this looks good. How about adding some unit tests before ch

Re: Review Request 23702: Patch for KAFKA-1070

> On Nov. 23, 2014, 8:35 p.m., Neha Narkhede wrote: > > core/src/main/scala/kafka/utils/ZkUtils.scala, line 714 > > > > > > Why not use persistent sequential nodes instead? if I used createPersistentSequential it keep

Re: Review Request 23702: Patch for KAFKA-1070

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23702/ --- (Updated Nov. 26, 2014, 4:29 a.m.) Review request for kafka. Bugs: KAFKA-1070

Re: Review Request 23702: Patch for KAFKA-1070

> On Nov. 23, 2014, 8:35 p.m., Neha Narkhede wrote: > > core/src/main/scala/kafka/common/GenerateBrokerIdException.scala, line 23 > > > > > > Given InconsistentBrokerIdException, looks like we don't need this. Generate

Re: Review Request 23702: Patch for KAFKA-1070

> On Nov. 23, 2014, 8:35 p.m., Neha Narkhede wrote: > > core/src/main/scala/kafka/server/KafkaConfig.scala, line 78 > > > > > > Why do we need a policy? It looks like we are trying to design the > > automatic broker id

Re: Review Request 23702: Patch for KAFKA-1070

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23702/#review62737 --- core/src/main/scala/kafka/common/GenerateBrokerIdException.scala

Re: Review Request 23702: Patch for KAFKA-1070

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

Re: Review Request 23702: Patch for KAFKA-1070

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23702/ --- (Updated Aug. 21, 2014, 5:26 p.m.) Review request for kafka. Bugs: KAFKA-1070

Re: Review Request 23702: Patch for KAFKA-1070

> On July 24, 2014, 1:41 a.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/KafkaServer.scala, lines 359-369 > > > > > > Does this cover the case for metaBrokerIdSet.size == 1 and brokerId < > > 0? In this case

Re: Review Request 23702: Patch for KAFKA-1070

> On July 24, 2014, 1:21 a.m., Neha Narkhede wrote: > > core/src/main/scala/kafka/utils/ZkUtils.scala, line 133 > > > > > > It will be good to refer to 1000 as a static variable inside > > KafkaConfig eg. KafkaConfig.

Re: Review Request 23702: Patch for KAFKA-1070

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23702/ --- (Updated July 25, 2014, 4:05 a.m.) Review request for kafka. Bugs: KAFKA-1070

Re: Review Request 23702: Patch for KAFKA-1070

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23702/ --- (Updated July 25, 2014, 3:58 a.m.) Review request for kafka. Bugs: KAFKA-1070

Re: Review Request 23702: Patch for KAFKA-1070

> On July 24, 2014, 1:41 a.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/KafkaServer.scala, lines 370-371 > > > > > > This probably only needs to be done after line 365? I was thinking of a case where a user

Re: Review Request 23702: Patch for KAFKA-1070

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23702/#review48608 --- core/src/main/scala/kafka/server/KafkaServer.scala

Re: Review Request 23702: Patch for KAFKA-1070

> On July 24, 2014, 1:21 a.m., Neha Narkhede wrote: > > core/src/main/scala/kafka/server/KafkaServer.scala, line 96 > > > > > > any reason you want to move this here from the beginning of the class? if the user didn't

Re: Review Request 23702: Patch for KAFKA-1070

> On July 24, 2014, 1:21 a.m., Neha Narkhede wrote: > > core/src/main/scala/kafka/utils/ZkUtils.scala, line 718 > > > > > > Shouldn't we be using createPersistentSequential? if I used createPersistentSequential it kee

Re: Review Request 23702: Patch for KAFKA-1070

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23702/#review48598 --- core/src/main/scala/kafka/server/KafkaServer.scala

Re: Review Request 23702: Patch for KAFKA-1070

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23702/ --- (Updated July 22, 2014, 6:34 p.m.) Review request for kafka. Bugs: KAFKA-1070

Re: Review Request 23702: Patch for KAFKA-1070

> On July 19, 2014, 11:28 p.m., Timothy Chen wrote: > > core/src/main/scala/kafka/server/KafkaConfig.scala, line 64 > > > > > > Why reduce the range to 1000? > > > > Sriharsha Chintalapani wrote: > As we go with a

Re: Review Request 23702: Patch for KAFKA-1070

> On July 19, 2014, 11:28 p.m., Timothy Chen wrote: > > core/src/main/scala/kafka/server/KafkaConfig.scala, line 64 > > > > > > Why reduce the range to 1000? > > As we go with auto-generating broker id we want to have

Re: Review Request 23702: Patch for KAFKA-1070

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23702/#review48184 --- core/src/main/scala/kafka/server/KafkaConfig.scala

Re: Review Request 23702: Patch for KAFKA-1070

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23702/ --- (Updated July 19, 2014, 11:06 p.m.) Review request for kafka. Bugs: KAFKA-107

Review Request 23702: Patch for KAFKA-1070

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