Hi Mickael, Thanks for the update. It answered my questions!
Thank you. Luke On Wed, Mar 30, 2022 at 12:09 AM Mickael Maison <mickael.mai...@gmail.com> wrote: > 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 > > > > > > > > > > > > > > > >
