> On May 27, 2014, 9:10 p.m., Neha Narkhede wrote: > > core/src/main/scala/kafka/utils/ZkUtils.scala, line 390 > > <https://reviews.apache.org/r/21899/diff/1/?file=594246#file594246line390> > > > > This is a generic API for conditional updates. Logic specific to Kafka > > shouldn't go here. Let's move it to a utils class. We have utils for Zk, > > API, clients but not for replication. Let's create a ReplicationUtils and > > add a common updateIsr utility. Controller and server can use that. Let's > > file a JIRA to refactor some existing utils related to replication to go to > > ReplicationUtils > > Sriharsha Chintalapani wrote: > I am working on adding ReplicationUtils class but for > ZkUtils.conditionalUpdatePersistenPath doesn't throw any exception it only > returns false,-1 incase of failures. Ideally I would like to check if the > current data in zk is same as the new data that we are trying to update only > when there is zkbadversionexception happens . What you think is the best way > todo that without modifying the ZkUtils methods. I can ofcourse check for > this whenever there is false returned from conditionalupdate but that doesn't > seems to be efficient. > > Neha Narkhede wrote: > We can include the logic of ignoring the update on a > ZkBadVersionException inside conditionalUpdatePersistentPath itself since it > makes sense for all uses cases of that API.
I moved updateIsr to ReplicationUtils. On a false return from ZkUtils.conditionalUpdatePersistentPath I am checking to see if the new Leader data same as zookeeper data except version if they are equal returning true and zookeeper data version. regarding updateIsr it uses Partition.scala data like topic,partitionId,brokerId etc.. and its only being called from Partition itself so not sure if it should move to ReplicationUtils. - Sriharsha ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21899/#review44043 ----------------------------------------------------------- On May 31, 2014, 10:50 p.m., Sriharsha Chintalapani wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/21899/ > ----------------------------------------------------------- > > (Updated May 31, 2014, 10:50 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1382 > https://issues.apache.org/jira/browse/KAFKA-1382 > > > Repository: kafka > > > Description > ------- > > KAFKA-1382. Update zkVersion on partition state update failures. > > > Diffs > ----- > > core/src/main/scala/kafka/cluster/Partition.scala > 518d2df5ae702d8c0937e1f9603fd11a54e24be8 > core/src/main/scala/kafka/controller/KafkaController.scala > e776423b8a38da6f08b2262c8141abf2064d37d2 > core/src/main/scala/kafka/utils/ReplicationUtils.scala PRE-CREATION > > Diff: https://reviews.apache.org/r/21899/diff/ > > > Testing > ------- > > > Thanks, > > Sriharsha Chintalapani > >
