To clarify slightly, the case described in the KIP doesn’t necessitate an extra mapping to the CZXID. But there is an issue filed against the controller, which would also affect the LeaderGeneration correctness. The suggested fix for this includes such a mapping, according to Jun’s reasoning in the Jira comments: https://issues.apache.org/jira/browse/KAFKA-1120 <https://issues.apache.org/jira/browse/KAFKA-1120>. Strictly speaking this is a separate issue though and I’ve updated the KIP accordingly.
B Ben Stopford Confluent, http://www.confluent.io <http://www.confluent.io/> > On 14 Dec 2016, at 11:37, Ben Stopford <b...@confluent.io> wrote: > > Thanks Onur. That’s correct, we no longer nee that extra mapping. I’ll update > the KIP. > B > > Ben Stopford > Confluent, http://www.confluent.io <http://www.confluent.io/> > > > >> On 11 Dec 2016, at 23:54, Onur Karaman <onurkaraman.apa...@gmail.com >> <mailto:onurkaraman.apa...@gmail.com>> wrote: >> >> Pretty happy to see a KIP tackling this problem! One comment below. >> >> The "Extending LeaderEpoch to include Returning Leaders" states: >> "To protect against this eventuality the controller will maintain a cached >> mapping of [broker -> Zookeeper CZXID] (CZXID is a unique and monotonic >> 64-bit number) for the broker’s registration in Zookeeper >> (/brokers/ids/[brokerId]). If the controller receives a Broker Registration >> where the CZXID has changed it will increment the Leader Epoch and >> propagate that value to the broker via the Leader and ISR Request (in the >> normal way), then update the cached CZXID for that broker." >> >> In general I think kafka underutilizes zookeeper's various flavors of zxids >> but this time it's not clear to me what the motivation is for maintaining >> the broker to czxid mapping. It seems that the following check is >> redundant: "If the controller receives a Broker Registration where the >> CZXID has changed". By definition, the czxid of the /brokers/ids/[brokerId] >> znode will always change upon successful broker registration ( >> https://zookeeper.apache.org/doc/r3.4.8/zookeeperProgrammers.html#sc_zkStatStructure >> >> <https://zookeeper.apache.org/doc/r3.4.8/zookeeperProgrammers.html#sc_zkStatStructure>). >> Why maintain the mapping at all? Why not just always update leader epochs >> and propagate every time the controller receives the broker registration zk >> event? >> >> On Sun, Dec 11, 2016 at 2:30 PM, Neha Narkhede <n...@confluent.io >> <mailto:n...@confluent.io>> wrote: >> >>> Good to see this KIP being proposed. Back when I added the epoch to the >>> replication protocol, we discussed adding it to the log due to the failure >>> scenarios listed in the KIP but I failed to convince people that it was >>> worth the effort needed to upgrade the cluster (especially after we asked >>> people to go through a painful backwards incompatible upgrade for 0.8 :-)) >>> The lack of including the leader epoch/generation in the log has also been >>> one of the biggest critiques of Kafka's replication protocol by the >>> distributed systems community. >>> >>> I'm in favor of this work though I think we shouldn't end up with 2 notions >>> of representing a leader's generation. When we added the epoch, we wanted >>> to add it to the log but we didn't. Now that we are adding the generation >>> id to the log, I think we should revisit calling it the epoch at all. Have >>> you thought about a way to evolve the epoch to the generation id throughout >>> and what it will take? >>> >>> On Sun, Dec 11, 2016 at 4:31 AM Ben Stopford <b...@confluent.io >>> <mailto:b...@confluent.io>> wrote: >>> >>>> Hi All >>>> >>>> Please find the below KIP which describes a proposed solution to a couple >>>> of issues that have been observed with the replication protocol. >>>> >>>> In short, the proposal replaces the use of the High Watermark, for >>>> follower log trunctation, with an alternate Generation Marker. This >>>> uniquely defines which leader messages were acknowledged by. >>>> >>>> >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP- >>>> <https://cwiki.apache.org/confluence/display/KAFKA/KIP-> >>> 101+-+Alter+Replication+Protocol+to+use+Leader+ >>> Generation+rather+than+High+Watermark+for+Truncation >>>> < >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP- >>>> <https://cwiki.apache.org/confluence/display/KAFKA/KIP-> >>> 101+-+Alter+Replication+Protocol+to+use+Leader+ >>> Generation+rather+than+High+Watermark+for+Truncation >>>>> >>>> >>>> All comments and suggestions greatly appreciated. >>>> >>>> Ben Stopford >>>> Confluent, http://www.confluent.io <http://www.confluent.io/> >>>> <http://www.confluent.io/ <http://www.confluent.io/>> >>>> >>>> -- >>> Thanks, >>> Neha >>> >
