Thanks for the KIP. Overall a good addition.
I am actually not sure if we need to add a new class? From my
understanding, if there is exactly one abstract method, the interface is
still functional? Thus, we could add a new method to
`StreamsPartitioner` with a default implementation (that calls the
existing `partition()` method and wraps the result into a singleton list)?
Not sure what the pros/cons for both approaches would be?
If we really add a new class, I am wondering if it should inherit from
`StreamsPartitioner` at all? Or course, if it does not, it's more stuff
we need to change, but the proposed overwrite of `partition()` that
throws also does not seem to be super clean? -- I am even wondering if
we should aim to deprecate the existing `partition()` and only offer
`partitions()` in the future?
For the broadcast option, I am wondering if returning `null` (not an
singleton with `-1`) might be a clear option to encode it? Empty list
would still be valid as "send to no partition".
Btw: The `StreamPartitioner` interface is also used for IQ. For both IQ
and FK-join, it seem ok to just add a runtime check that the returned
list is a singleton (in case we don't add a new class)?
-Matthias
On 6/26/22 7:55 AM, Sagar wrote:
Hi Florin,
Thanks for the comment! You brought up a very good point.. Actually I was
focussed too much on the multicast operation and didn't pay attention to
the other places where StramPartitioner is being used. TBH, I wasn't even
aware that the StreamPartitioner is being used for FK joins so thanks
definitely for pointing that out!
Regarding how we handle that, I think since the FK join uses the partition
number info for subscription/message passing semantics, I would basically
like to propose that we can throw an Exception when a user tries to pass
an object which is an instance of MulticastPartitioner. This would keep
things simple IMO because adding multicast keys to FK would just make it
all the more complicated.
Other than that, the usages/implementations of StreamPartitioner are on
tests which would be taken care of if needed.
Let me know what you think.
Thanks!
Sagar.
On Sat, Jun 25, 2022 at 5:11 PM Florin Akermann <florin.akerm...@gmail.com>
wrote:
Hi Sagar,
Thanks for the KIP.
I am wondering about the following. What about other classes than the
org.apache.kafka.streams.processor.internals.RecordCollectorImpl. Would
they still call .partition(...) and just crash? Or is it a given that they
never receive a reference to a Partitioner of
type MultiCastStreamPartitioner?
Florin
On Sat, 28 May 2022 at 05:44, Sagar <sagarmeansoc...@gmail.com> wrote:
Hi All,
I’m thinking to move this KIP to vote section if there aren’t any
objections.
Plz let me know if I that’s ok.
Thanks!
Sagar.
On Tue, 24 May 2022 at 10:32 PM, Sagar <sagarmeansoc...@gmail.com>
wrote:
Hi All,
Bumping this discussion thread again to see if there are any
comments/concerns on this.
Thanks!
Sagar.
On Wed, May 18, 2022 at 11:44 PM Sagar <sagarmeansoc...@gmail.com>
wrote:
Hi All,
I would like to open a discussion thread on
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=211883356
.
Thanks!
Sagar.
