No worries. Thanks for fixing it! C.
On Tue, Jun 25, 2019, at 13:47, Justine Olshan wrote: > Also apologies on the late link to the jira, but apparently https links do > not work and it kept defaulting to an image on my desktop even when it > looked like I put the correct link in. Weird... > > On Tue, Jun 25, 2019 at 1:41 PM Justine Olshan <jols...@confluent.io> wrote: > > > I came up with a good solution for this and will push the commit soon. The > > repartition will be called only when a partition is not manually sent. > > > > On Tue, Jun 25, 2019 at 1:39 PM Colin McCabe <cmcc...@apache.org> wrote: > > > >> Well, this is a generic partitioner method, so it shouldn't dictate any > >> particular behavior. > >> > >> Colin > >> > >> > >> On Tue, Jun 25, 2019, at 12:04, Justine Olshan wrote: > >> > I also just noticed that if we want to use this method on the keyed > >> record > >> > case, I will need to move the method outside of the sticky (no key, no > >> set > >> > partition) check. Not a big problem, but something to keep in mind. > >> > Perhaps, we should encapsulate the sticky vs. not behavior inside the > >> > method? More things to think about. > >> > > >> > On Tue, Jun 25, 2019 at 11:55 AM Colin McCabe <cmcc...@apache.org> > >> wrote: > >> > > >> > > Hi Justine, > >> > > > >> > > The KIP discusses adding a new method to the partitioner interface. > >> > > > >> > > > default public Integer onNewBatch(String topic, Cluster cluster) { > >> ... } > >> > > > >> > > However, this new method doesn't give the partitioner access to the > >> key > >> > > and value of the message. While this works for the case described > >> here (no > >> > > key), in general we might need this information when re-assigning a > >> > > partitition based on the batch completing. So I think we should add > >> these > >> > > methods to onNewBatch. > >> > > > >> > > Also, it would be nice to call this something like > >> "repartitionOnNewBatch" > >> > > or something, to make it clearer what is going on. > >> > > > >> > > best, > >> > > Colin > >> > > > >> > > On Mon, Jun 24, 2019, at 18:32, Boyang Chen wrote: > >> > > > Thank you Justine for the KIP! Do you mind creating a corresponding > >> JIRA > >> > > > ticket too? > >> > > > > >> > > > On Mon, Jun 24, 2019 at 4:51 PM Colin McCabe <cmcc...@apache.org> > >> wrote: > >> > > > > >> > > > > Hi Justine, > >> > > > > > >> > > > > Thanks for the KIP. This looks great! > >> > > > > > >> > > > > In one place in the KIP, you write: "Remove > >> > > > > testRoundRobinWithUnavailablePartitions() and testRoundRobin() > >> since > >> > > the > >> > > > > round robin functionality of the partitioner has been removed." > >> You > >> > > can > >> > > > > skip this and similar lines. We don't need to describe changes to > >> > > internal > >> > > > > test classes in the KIP since they're not visible to users or > >> external > >> > > > > developers. > >> > > > > > >> > > > > It seems like maybe the performance tests should get their own > >> section. > >> > > > > Right now, the way the layout is makes it look like they are part > >> of > >> > > the > >> > > > > "Compatibility, Deprecation, and Migration Plan" > >> > > > > > >> > > > > best, > >> > > > > Colin > >> > > > > > >> > > > > > >> > > > > On Mon, Jun 24, 2019, at 14:04, Justine Olshan wrote: > >> > > > > > Hello, > >> > > > > > This is the discussion thread for KIP-480: Sticky Partitioner. > >> > > > > > > >> > > > > > > >> > > > > > >> > > > >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-480%3A+Sticky+Partitioner > >> > > > > > > >> > > > > > Thank you, > >> > > > > > Justine Olshan > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > > >