----------------------------------------------------------- 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 <https://reviews.apache.org/r/23702/#comment85309> any reason you want to move this here from the beginning of the class? core/src/main/scala/kafka/server/KafkaServer.scala <https://reviews.apache.org/r/23702/#comment85311> uses it *as* the brokerId core/src/main/scala/kafka/server/KafkaServer.scala <https://reviews.apache.org/r/23702/#comment85319> If there is another exception (other than FileNotFound which we should ignore) while reading the file, it will be good to log a meaningful error before rethrowing it. core/src/main/scala/kafka/server/KafkaServer.scala <https://reviews.apache.org/r/23702/#comment85314> we should also fsync the file to disk and close out the file output stream core/src/main/scala/kafka/utils/ZkUtils.scala <https://reviews.apache.org/r/23702/#comment85320> It will be good to refer to 1000 as a static variable inside KafkaConfig eg. KafkaConfig.MinimumBrokerSequenceId core/src/main/scala/kafka/utils/ZkUtils.scala <https://reviews.apache.org/r/23702/#comment85322> this logic should ideally be encapsulated under updatePersistentSequentialPath inside ZkUtils. core/src/main/scala/kafka/utils/ZkUtils.scala <https://reviews.apache.org/r/23702/#comment85323> Shouldn't we be using createPersistentSequential? core/src/test/scala/unit/kafka/server/ServerGenerateBrokerIdTest.scala <https://reviews.apache.org/r/23702/#comment85325> can we add more test cases that cover all possible combinations of using the config based broker id and the zk based one? It will also help to explicitly separate some of these test cases. - Neha Narkhede On July 22, 2014, 6:34 p.m., Sriharsha Chintalapani wrote: > > ----------------------------------------------------------- > 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 > https://issues.apache.org/jira/browse/KAFKA-1070 > > > Repository: kafka > > > Description > ------- > > KAFKA-1070. Auto assign node id. > > > KAFKA-1070. Auto-assign node id. > > > Diffs > ----- > > core/src/main/scala/kafka/common/InconsistentBrokerIdException.scala > PRE-CREATION > core/src/main/scala/kafka/server/KafkaConfig.scala > 50b09edb73af1b45f88f919ac8c46ae056878c8e > core/src/main/scala/kafka/server/KafkaServer.scala > def1dc2a5818d45d9ee0881137ff989cec4eb9b1 > core/src/main/scala/kafka/utils/ZkUtils.scala > dcdc1ce2b02c996294e19cf480736106aaf29511 > core/src/test/scala/unit/kafka/server/ServerGenerateBrokerIdTest.scala > PRE-CREATION > core/src/test/scala/unit/kafka/utils/TestUtils.scala > 3faa884f8eb83c7c00baab416d0acfb488dc39c1 > > Diff: https://reviews.apache.org/r/23702/diff/ > > > Testing > ------- > > > Thanks, > > Sriharsha Chintalapani > >
