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 > > > > > > > > >
