Hi Luke, Thanks for the feedback.
1. Thanks, fixed! 2. Yes that's right. It's the same behavior for topic policies 3. I've added details about how the mentioned scenarios could be addressed. The information required to make such decisions is not part of the Kafka cluster metadata so an external input is necessary. This KIP does not propose a specific mechanism for doing it. I hope this answers your questions. Thanks, Mickael On Tue, Mar 29, 2022 at 5:42 PM Mickael Maison <mickael.mai...@gmail.com> wrote: > > Hi Ryanne, > > That's a good point! > > There's no value in having all implementations perform the same sanity > checks. If the replication factor is < 1 or larger than the current > number of registered brokers, the controller should directly throw > InvalidReplicationFactorException and not call the ReplicaPlacer. I've > updated the KIP so the place() method now only throws > ReplicaPlacementException. > > Thanks, > Mickael > > > > On Mon, Mar 28, 2022 at 6:20 PM Ryanne Dolan <ryannedo...@gmail.com> wrote: > > > > Wondering about InvalidReplicationFactorException. Why would an > > implementation throw this? Given the information passed to the method, > > seems like this could only be thrown if there were obviously invalid > > arguments, like a negative number or zero. Can we just guarantee such > > invalid arguments aren't passed in? > > > > Ryanne > > > > On Sat, Mar 26, 2022, 8:51 AM Luke Chen <show...@gmail.com> wrote: > > > > > Hi Mickael, > > > > > > Thanks for the KIP! > > > It's indeed a pain point for the Kafka admins. > > > > > > I have some comments: > > > 1. Typo in motivation section: When administrators [when to] remove > > > brokers > > > from a cluster,.... > > > 2. If different `replica.placer.class.name` configs are set in all > > > controllers, I think only the config for "active controller" will take > > > effect, right? > > > 3. Could you explain more about how the proposal fixes some scenarios you > > > listed, ex: the new added broker case. How could we know the broker is new > > > added? I guess it's by checking the broker load via some metrics > > > dynamically, right? > > > > > > > > > Thank you. > > > Luke > > > > > > On Fri, Mar 18, 2022 at 10:30 AM Ryanne Dolan <ryannedo...@gmail.com> > > > wrote: > > > > > > > Thanks Mickael, this makes sense to me! I've been wanting something like > > > > this in order to decommission a broker without new partitions getting > > > > accidentally assigned to it. > > > > > > > > Ryanne > > > > > > > > On Thu, Mar 17, 2022, 5:56 AM Mickael Maison <mickael.mai...@gmail.com> > > > > wrote: > > > > > > > > > Hi, > > > > > > > > > > I'd like to start a new discussion on KIP-660. I originally wrote this > > > > > KIP in 2020 and the initial discussion > > > > > (https://lists.apache.org/thread/xn7xyb74nyt281brto4x28r9rzxm4lp9) > > > > > raised some concerns especially around KRaft (which did not exist at > > > > > that time) and scalability. > > > > > > > > > > Since then, we got a new KRaft controller so I've been able to revisit > > > > > this KIP. I kept the KIP number as it's essentially the same idea, but > > > > > the proposal is significantly different: > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-660%3A+Pluggable+ReplicaPlacer > > > > > > > > > > Please take a look and let me know if you have any feedback. > > > > > > > > > > Thanks, > > > > > Mickael > > > > > > > > > > > >
