> On Jan. 9, 2015, 11:39 p.m., Neha Narkhede wrote:
> > core/src/main/scala/kafka/server/BrokerMetadataFileHandler.scala, line 48
> > <https://reviews.apache.org/r/23702/diff/8-9/?file=776402#file776402line48>
> >
> >     Does java serialization for the Properties object allow you to read 
> > just the version field? Basically, if we want to upgrade the version of 
> > this file, for some time the server would have to support 2 versions and 
> > read the file differently based on the different versions.
> >     
> >     Also, if you serialize the file this way, is it still human readable? 
> >     
> >     I'd recommend you look at other checkpoint file and follow the same or 
> > change those to use the new strategy you pick.
> 
> Sriharsha Chintalapani wrote:
>     brokerMetaProps.store(fileOutputStream,"") stores the data in 
> human-readable form . For ex:
>     "#Mon Jan 12 10:10:46 PST 2015
>     version=0
>     broker.id=1001"
>     In BrokerMetadataCheckpoint.read() methods checks the version and does 
> case operation similar to OffsetCheckpoint.
>     
>     In storing data  OffsetCheckpoint is different, OffsetCheckpoint just 
> stores the values like
>     0
>     topic=offset
>     
>     I like the current version for broker metadata as the file will be 
> self-explanatory if users needed to change or look at the current values.
>     I don't think OffsetCheckpoint and BrokerMetadataCheckpoint needs to 
> follow same format to store data. If you disagree I can change it to just 
> store data like in Offsetcheckpoint.
> 
> Neha Narkhede wrote:
>     Okay. What do you think of the following -
>     "Does java serialization for the Properties object allow you to read just 
> the version field? Basically, if we want to upgrade the version of this file, 
> for some time the server would have to support 2 versions and read the file 
> differently based on the different versions."
> 
> Sriharsha Chintalapani wrote:
>     I am not sure if I understand correctly. BrokerMetadataCheckpoint.read() 
> does this.
>             val brokerMetaProps = new 
> VerifiableProperties(Utils.loadProps(file.getAbsolutePath()))
>             val version = brokerMetaProps.getIntInRange("version", (0, 
> Int.MaxValue))
>             version match {
>               case 0 => 
>               .
>     It does load the entire contents into brokerMetaProps and we can pick the 
> version from it. 
>     If the intention is to just read the single line to get the version its 
> not possible with the current approach.
>     Any specific reason that we need just the version. In either case we will 
> be able to support multiple version files at the same.
>     Since the file contents are like key=value pairs.

> It does load the entire contents into brokerMetaProps and we can pick the 
> version from it.

You are right. This should be sufficient. The broker can be smart about 
interpreting the rest of the fields based on the read version.


- Neha


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23702/#review67551
-----------------------------------------------------------


On Jan. 12, 2015, 6:46 p.m., Sriharsha Chintalapani wrote:
> 
> -----------------------------------------------------------
> 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
>     https://issues.apache.org/jira/browse/KAFKA-1070
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1070. Auto-assign node id.
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/common/GenerateBrokerIdException.scala 
> PRE-CREATION 
>   core/src/main/scala/kafka/common/InconsistentBrokerIdException.scala 
> PRE-CREATION 
>   core/src/main/scala/kafka/server/BrokerMetadataCheckpoint.scala 
> PRE-CREATION 
>   core/src/main/scala/kafka/server/KafkaConfig.scala 
> 6e26c5436feb4629d17f199011f3ebb674aa767f 
>   core/src/main/scala/kafka/server/KafkaServer.scala 
> 1691ad7fc80ca0b112f68e3ea0cbab265c75b26b 
>   core/src/main/scala/kafka/utils/ZkUtils.scala 
> 56e3e88e0cc6d917b0ffd1254e173295c1c4aabd 
>   core/src/test/scala/unit/kafka/server/ServerGenerateBrokerIdTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/utils/TestUtils.scala 
> c9e8ba257b77f46c5c9b62b451470348b6e58889 
> 
> Diff: https://reviews.apache.org/r/23702/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sriharsha Chintalapani
> 
>

Reply via email to