The vote for this KIP passes with the following results: * Three binding +1 votes from Colin, Guozhang, and Jason * Two non-binding +1 votes from Jose and Boyang * No +0 or -1 votes
Thanks, everyone! -David On Tue, Jun 2, 2020 at 8:56 PM Jason Gustafson <ja...@confluent.io> wrote: > +1 I agree with Guozhang that broker epoch will need a separate discussion. > > Thanks! > Jason > > On Thu, May 28, 2020 at 9:34 AM Guozhang Wang <wangg...@gmail.com> wrote: > > > David, thanks for the KIP. I'm +1 on it as well. > > > > One note is that in post-ZK world, we would need a different way to get > > broker epoch since it is updated as ZKversion today. I believe we would > > have this discussion in a different KIP though. > > > > > > Guozhang > > > > On Wed, May 27, 2020 at 8:26 PM Colin McCabe <cmcc...@apache.org> wrote: > > > > > Thanks, David. +1 (binding). > > > > > > cheers, > > > Colin > > > > > > On Wed, May 27, 2020, at 18:21, David Arthur wrote: > > > > Colin, thanks for the feedback. Good points. I've updated the KIP > with > > > your > > > > suggestions. > > > > > > > > -David > > > > > > > > On Wed, May 27, 2020 at 4:05 PM Colin McCabe <cmcc...@apache.org> > > wrote: > > > > > > > > > Hi David, > > > > > > > > > > Thanks for the KIP! > > > > > > > > > > The KIP refers to "the KIP-500 bridge release (version 2.6.0 as of > > the > > > > > time of this proposal)". This is out of date-- the bridge release > > > will be > > > > > one of the 3.x releases. We should either update this to 3.0, or > > > perhaps > > > > > just take out the reference to a specific version, since it's not > > > necessary > > > > > to understand the rest of the KIP. > > > > > > > > > > > ... and potentially could replace the existing controlled > shutdown > > > RPC. > > > > > Since this RPC > > > > > > is somewhat generic, it could also be used to mark a replicas a > > > "online" > > > > > following some > > > > > > kind of log dir recovery procedure (out of scope for this > > proposal). > > > > > > > > > > I think it would be good to move this part into the "Future Work" > > > section. > > > > > > > > > > > The Reason field is an optional textual description of why the > > event > > > is > > > > > being sent > > > > > > > > > > Since we implemented optional fields in KIP-482, describing this > > field > > > as > > > > > "optional" might be confusing. Probably better to avoid describing > > it > > > that > > > > > way, unless it's a tagged field. > > > > > > > > > > > - If no Topic is given, it is implied that all topics on this > > broker > > > are > > > > > being indicated > > > > > > - If a Topic and no partitions are given, it is implied that all > > > > > partitions of this topic are being indicated > > > > > > > > > > I would prefer to leave out these "shortcuts" since they seem > likely > > to > > > > > lead to confusion and bugs. > > > > > > > > > > For example, suppose that the controller has just created a new > > > partition > > > > > for topic "foo" and put it on broker 3. But then, before broker 3 > > > gets the > > > > > LeaderAndIsrRequest from the controller, broker 3 get a bad log > > > directory. > > > > > So it sends an AlterReplicaStateRequest to the controller > specifying > > > topic > > > > > foo and leaving out the partition list (using the first > "shortcut".) > > > The > > > > > new partition will get marked as offline even though it hasn't even > > > been > > > > > created, much less assigned to the bad log directory. > > > > > > > > > > Since log directory failures are rare, spelling out the full set of > > > > > affected partitions when one happens doesn't seem like that much > of a > > > > > burden. This is also consistent with what we currently do. In > fact, > > > it's > > > > > much more efficient than what we currently do, since with KIP-589, > we > > > won't > > > > > have to enumerate partitions that aren't on the failed log > directory. > > > > > > > > > > In the future work section: If we eventually want to replace > > > > > ControlledShutdownRequest with this RPC, we'll need some additional > > > > > functionality. Specifically, we'll need the ability to tell the > > > controller > > > > > to stop putting new partitions on the broker that sent the request. > > > That > > > > > could be done with a separate request or possibly additional flags > on > > > this > > > > > request. In any case, we don't have to solve that problem now. > > > > > > > > > > Thanks again for the KIP... great to see this moving forward. > > > > > > > > > > regards, > > > > > Colin > > > > > > > > > > > > > > > On Wed, May 20, 2020, at 12:22, David Arthur wrote: > > > > > > Hello, all. I'd like to start the vote for KIP-589 which proposes > > to > > > add > > > > > a > > > > > > new AlterReplicaState RPC. > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-589+Add+API+to+update+Replica+state+in+Controller > > > > > > > > > > > > Cheers, > > > > > > David > > > > > > > > > > > > > > > > > > > > > > > -- > > > > -David > > > > > > > > > > > > > -- > > -- Guozhang > > > -- David Arthur
