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