> On Oct. 7, 2014, 12:15 a.m., Guozhang Wang wrote: > > Since now the first iteration of "if" statements is only used for logging, > > could we just merge it into the second check? > > Jiangjie Qin wrote: > Guozhang, thanks for the review. I actually thought about it before. I > agree that the code looks simpler if we just call partition.makeFollower > without checking whether leader is up or not. The reason I retained the first > if statement is that it seems more reasonable to create the remote replicas > only when the leader broker is online, which is done in > partition.makeFollower. And I'm not 100% sure whether it matters if we just > create the remote replicas objects without checking the liveliness of leader > broker.
The checking of the liveness of the leader is just for assertation (i.e. the leader should always be alive when the leaderAndISR request is received, otherwise something bad has happended and we need to log the errors, but still proceed by skiping this request). - Guozhang ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26373/#review55615 ----------------------------------------------------------- On Oct. 6, 2014, 5:06 p.m., Jiangjie Qin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26373/ > ----------------------------------------------------------- > > (Updated Oct. 6, 2014, 5:06 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1647 > https://issues.apache.org/jira/browse/KAFKA-1647 > > > Repository: kafka > > > Description > ------- > > Fix for Kafka-1647. > > > Diffs > ----- > > core/src/main/scala/kafka/server/ReplicaManager.scala > 78b7514cc109547c562e635824684fad581af653 > > Diff: https://reviews.apache.org/r/26373/diff/ > > > Testing > ------- > > > Thanks, > > Jiangjie Qin > >
