You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by "Matthias J. Sax" <ma...@confluent.io> on 2017/12/04 22:57:20 UTC
Re: [DISCUSS]: KIP-230: Name Windowing Joins
Matthias,
thanks for the KIP.
Can you please c&p the corresponding content instead of just putting
links? A KIP should be a self-contained Wiki page.
Also, if we add a optional config parameter, how would we specify it?
Please list all changes to want to apply to `Joined` class.
Furthermore, `Joined` is also used for KStream-KTable join but the KIP
only talks about windowed joins (ie, KStream-KTream join). What the
impact on KStream-KTable join?
-Matthias
On 11/29/17 6:09 AM, Matt Farmer wrote:
> Hi Matthias,
>
> I certainly have found the auto-generated names unwieldy while doing
> cluster administration.
>
> I will point out that your KIP doesn't outline what would happen if you
> picked a name that resulted in a non unique topic name? What would be the
> error handling behavior there?
>
> On Wed, Nov 29, 2017 at 9:03 AM Matthias Margush <ma...@gmail.com>
> wrote:
>
>> Hi everyone,
>>
>> I created this KIP to allow windowing joins to be named. If named, then the
>> associated internal topic names would be derived from that, instead of
>> being randomly generated.
>>
>>
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+230%3A+Name+Windowing+Joins
>>
>> Thanks,
>>
>> Matthias
>>
>
Re: [DISCUSS]: KIP-230: Name Windowing Joins
Posted by "Matthias J. Sax" <ma...@confluent.io>.
With KIP-372 accepted, we can retire this KIP, because it's subsumed.
https://cwiki.apache.org/confluence/display/KAFKA/KIP-372%3A+Naming+Repartition+Topics+for+Joins+and+Grouping
I'll update the wiki pages accordingly. Thanks for initiating this KIP
Florian!
-Matthias
On 6/10/18 5:24 PM, Matthias J. Sax wrote:
> What is the status of this KIP?
>
> I also want to point out, that it seems that there is some overlap with
> KIP-307 that was proposed recently (I cc'ed Florian who proposed
> KIP-307). KIP-230 might even be subsumed.
>
> I any case, it would be good to sync up and collaborate for both KIPs.
>
>
> -Matthias
>
>
> On 2/28/18 1:07 PM, Guozhang Wang wrote:
>> Hi Matthias,
>>
>> I've also made a pass over the KIP, aside from the-other-Matthias's
>> comment, I'm wondering if you have scenarios that want to distinguish the
>> two internal topics of the join?
>>
>> Currently we use "-this" and "-other" suffix for the topics. So for example:
>>
>> stream1.join(stream2, ...) // stream1 will be materialized with "-this",
>> and stream2 with "-other"
>>
>> While:
>>
>> stream2.join(stream1, ...) // stream2 will be materialized with "-this",
>> and stream1 with "-other"
>>
>>
>> If we think it is reasonable to require users be aware that the above join
>> situations are not exactly the same, then the current naming is fine; if we
>> want them to be mutually reused (I'm not sure if this is a common case?)
>> then we probably need to consider something new?
>>
>>
>>
>> Guozhang
>>
>>
>>
>>
>>
>> On Tue, Feb 13, 2018 at 7:02 PM, Matthias Margush <
>> matthias.margush@gmail.com> wrote:
>>
>>> Thanks for the reminder! I need to do some wordsmithing based on the
>>> feedback I’ve gotten. I’ll do that soon (hopefully).
>>> On Tue, Feb 13, 2018 at 1:45 PM Matthias J. Sax <ma...@confluent.io>
>>> wrote:
>>>
>>>> Is there any updates for this KIP?
>>>>
>>>> -Matthias
>>>>
>>>> On 12/28/17 12:27 PM, Matthias J. Sax wrote:
>>>>> Thanks for updating the KIP.
>>>>>
>>>>> The code-diff is a little hard to read. It's better to so something
>>>>> similar as in this KIP:
>>>>>
>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>>> 245%3A+Use+Properties+instead+of+StreamsConfig+in+KafkaStreams+constructor
>>>>>
>>>>> (Just as an example. Maybe take a look as other KIPs, too.)
>>>>>
>>>>> Some side remarks:
>>>>> - please update the link to the DISCUSS thread
>>>>> - there are some typos: Kstream -> KStream; Topology Builder exception
>>>>> -> TopologyBuilderException
>>>>>
>>>>>
>>>>> You propose to add `otherValueSerde(final String joinName)` -- I guess
>>>>> the method name is a c&p error and method name must be updated?
>>>>>
>>>>> Changes to internal classes like `KStreamImpl` are not required in the
>>>>> KIP as those as implementation details. The KIP should focus on public
>>>>> changes.
>>>>>
>>>>>
>>>>> -Matthias
>>>>>
>>>>>
>>>>> On 12/26/17 11:19 AM, Matthias Margush wrote:
>>>>>> Greetings.
>>>>>>
>>>>>> Thanks for the comments and suggestions. I updated the KIP with these
>>>>>> proposals for the questions posed by Matt & Matthias:
>>>>>>
>>>>>> *Can you please c&p the corresponding content instead of just
>>>>>> putting links? A KIP should be a self-contained Wiki page. Also, if we
>>>> add
>>>>>> a optional config parameter, how would we specify it? **Please list
>>> all
>>>>>> changes to want to apply to `Joined` class.*
>>>>>>
>>>>>> I added more details around the proposed changes directly to the KIP.
>>>>>>
>>>>>> *I will point out that your KIP doesn't outline what would happen if
>>>>>> you picked a name that resulted in a non unique topic name? What would
>>>> be
>>>>>> the error handling behavior there?*
>>>>>>
>>>>>> Looking at the current behavior of methods that allow the user to
>>>> specify
>>>>>> names for internal resources (e.g. `reduce`, `aggregate`), I added a
>>>>>> proposal that the code generate a similar exception if a name conflict
>>>> is
>>>>>> detected in the topology:
>>>>>>
>>>>>> org.apache.kafka.streams.errors.TopologyBuilderException: "Invalid
>>>> topology
>>>>>> building: Topic reduction-same-name-repartition has already been
>>>> registered
>>>>>> by another source."
>>>>>>
>>>>>> *What is the impact on KStream-KTable join?*
>>>>>>
>>>>>> Proposed that kstream-ktable joins similarly make use of the provided
>>>>>> joinName when generating internal repartition topics.
>>>>>>
>>>>>> On Mon, Dec 4, 2017 at 2:57 PM Matthias J. Sax <matthias@confluent.io
>>>>
>>>>>> wrote:
>>>>>>
>>>>>>> Matthias,
>>>>>>>
>>>>>>> thanks for the KIP.
>>>>>>>
>>>>>>> Can you please c&p the corresponding content instead of just putting
>>>>>>> links? A KIP should be a self-contained Wiki page.
>>>>>>>
>>>>>>> Also, if we add a optional config parameter, how would we specify it?
>>>>>>> Please list all changes to want to apply to `Joined` class.
>>>>>>>
>>>>>>> Furthermore, `Joined` is also used for KStream-KTable join but the
>>> KIP
>>>>>>> only talks about windowed joins (ie, KStream-KTream join). What the
>>>>>>> impact on KStream-KTable join?
>>>>>>>
>>>>>>>
>>>>>>> -Matthias
>>>>>>>
>>>>>>> On 11/29/17 6:09 AM, Matt Farmer wrote:
>>>>>>>> Hi Matthias,
>>>>>>>>
>>>>>>>> I certainly have found the auto-generated names unwieldy while doing
>>>>>>>> cluster administration.
>>>>>>>>
>>>>>>>> I will point out that your KIP doesn't outline what would happen if
>>>> you
>>>>>>>> picked a name that resulted in a non unique topic name? What would
>>> be
>>>> the
>>>>>>>> error handling behavior there?
>>>>>>>>
>>>>>>>> On Wed, Nov 29, 2017 at 9:03 AM Matthias Margush <
>>>>>>> matthias.margush@gmail.com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Hi everyone,
>>>>>>>>>
>>>>>>>>> I created this KIP to allow windowing joins to be named. If named,
>>>> then
>>>>>>> the
>>>>>>>>> associated internal topic names would be derived from that, instead
>>>> of
>>>>>>>>> being randomly generated.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
>>> 230%3A+Name+Windowing+Joins
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Matthias
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
>>
>
Re: [DISCUSS]: KIP-230: Name Windowing Joins
Posted by "Matthias J. Sax" <ma...@confluent.io>.
What is the status of this KIP?
I also want to point out, that it seems that there is some overlap with
KIP-307 that was proposed recently (I cc'ed Florian who proposed
KIP-307). KIP-230 might even be subsumed.
I any case, it would be good to sync up and collaborate for both KIPs.
-Matthias
On 2/28/18 1:07 PM, Guozhang Wang wrote:
> Hi Matthias,
>
> I've also made a pass over the KIP, aside from the-other-Matthias's
> comment, I'm wondering if you have scenarios that want to distinguish the
> two internal topics of the join?
>
> Currently we use "-this" and "-other" suffix for the topics. So for example:
>
> stream1.join(stream2, ...) // stream1 will be materialized with "-this",
> and stream2 with "-other"
>
> While:
>
> stream2.join(stream1, ...) // stream2 will be materialized with "-this",
> and stream1 with "-other"
>
>
> If we think it is reasonable to require users be aware that the above join
> situations are not exactly the same, then the current naming is fine; if we
> want them to be mutually reused (I'm not sure if this is a common case?)
> then we probably need to consider something new?
>
>
>
> Guozhang
>
>
>
>
>
> On Tue, Feb 13, 2018 at 7:02 PM, Matthias Margush <
> matthias.margush@gmail.com> wrote:
>
>> Thanks for the reminder! I need to do some wordsmithing based on the
>> feedback I’ve gotten. I’ll do that soon (hopefully).
>> On Tue, Feb 13, 2018 at 1:45 PM Matthias J. Sax <ma...@confluent.io>
>> wrote:
>>
>>> Is there any updates for this KIP?
>>>
>>> -Matthias
>>>
>>> On 12/28/17 12:27 PM, Matthias J. Sax wrote:
>>>> Thanks for updating the KIP.
>>>>
>>>> The code-diff is a little hard to read. It's better to so something
>>>> similar as in this KIP:
>>>>
>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>> 245%3A+Use+Properties+instead+of+StreamsConfig+in+KafkaStreams+constructor
>>>>
>>>> (Just as an example. Maybe take a look as other KIPs, too.)
>>>>
>>>> Some side remarks:
>>>> - please update the link to the DISCUSS thread
>>>> - there are some typos: Kstream -> KStream; Topology Builder exception
>>>> -> TopologyBuilderException
>>>>
>>>>
>>>> You propose to add `otherValueSerde(final String joinName)` -- I guess
>>>> the method name is a c&p error and method name must be updated?
>>>>
>>>> Changes to internal classes like `KStreamImpl` are not required in the
>>>> KIP as those as implementation details. The KIP should focus on public
>>>> changes.
>>>>
>>>>
>>>> -Matthias
>>>>
>>>>
>>>> On 12/26/17 11:19 AM, Matthias Margush wrote:
>>>>> Greetings.
>>>>>
>>>>> Thanks for the comments and suggestions. I updated the KIP with these
>>>>> proposals for the questions posed by Matt & Matthias:
>>>>>
>>>>> *Can you please c&p the corresponding content instead of just
>>>>> putting links? A KIP should be a self-contained Wiki page. Also, if we
>>> add
>>>>> a optional config parameter, how would we specify it? **Please list
>> all
>>>>> changes to want to apply to `Joined` class.*
>>>>>
>>>>> I added more details around the proposed changes directly to the KIP.
>>>>>
>>>>> *I will point out that your KIP doesn't outline what would happen if
>>>>> you picked a name that resulted in a non unique topic name? What would
>>> be
>>>>> the error handling behavior there?*
>>>>>
>>>>> Looking at the current behavior of methods that allow the user to
>>> specify
>>>>> names for internal resources (e.g. `reduce`, `aggregate`), I added a
>>>>> proposal that the code generate a similar exception if a name conflict
>>> is
>>>>> detected in the topology:
>>>>>
>>>>> org.apache.kafka.streams.errors.TopologyBuilderException: "Invalid
>>> topology
>>>>> building: Topic reduction-same-name-repartition has already been
>>> registered
>>>>> by another source."
>>>>>
>>>>> *What is the impact on KStream-KTable join?*
>>>>>
>>>>> Proposed that kstream-ktable joins similarly make use of the provided
>>>>> joinName when generating internal repartition topics.
>>>>>
>>>>> On Mon, Dec 4, 2017 at 2:57 PM Matthias J. Sax <matthias@confluent.io
>>>
>>>>> wrote:
>>>>>
>>>>>> Matthias,
>>>>>>
>>>>>> thanks for the KIP.
>>>>>>
>>>>>> Can you please c&p the corresponding content instead of just putting
>>>>>> links? A KIP should be a self-contained Wiki page.
>>>>>>
>>>>>> Also, if we add a optional config parameter, how would we specify it?
>>>>>> Please list all changes to want to apply to `Joined` class.
>>>>>>
>>>>>> Furthermore, `Joined` is also used for KStream-KTable join but the
>> KIP
>>>>>> only talks about windowed joins (ie, KStream-KTream join). What the
>>>>>> impact on KStream-KTable join?
>>>>>>
>>>>>>
>>>>>> -Matthias
>>>>>>
>>>>>> On 11/29/17 6:09 AM, Matt Farmer wrote:
>>>>>>> Hi Matthias,
>>>>>>>
>>>>>>> I certainly have found the auto-generated names unwieldy while doing
>>>>>>> cluster administration.
>>>>>>>
>>>>>>> I will point out that your KIP doesn't outline what would happen if
>>> you
>>>>>>> picked a name that resulted in a non unique topic name? What would
>> be
>>> the
>>>>>>> error handling behavior there?
>>>>>>>
>>>>>>> On Wed, Nov 29, 2017 at 9:03 AM Matthias Margush <
>>>>>> matthias.margush@gmail.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Hi everyone,
>>>>>>>>
>>>>>>>> I created this KIP to allow windowing joins to be named. If named,
>>> then
>>>>>> the
>>>>>>>> associated internal topic names would be derived from that, instead
>>> of
>>>>>>>> being randomly generated.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>
>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
>> 230%3A+Name+Windowing+Joins
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> Matthias
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
>
>
Re: [DISCUSS]: KIP-230: Name Windowing Joins
Posted by Guozhang Wang <wa...@gmail.com>.
Hi Matthias,
I've also made a pass over the KIP, aside from the-other-Matthias's
comment, I'm wondering if you have scenarios that want to distinguish the
two internal topics of the join?
Currently we use "-this" and "-other" suffix for the topics. So for example:
stream1.join(stream2, ...) // stream1 will be materialized with "-this",
and stream2 with "-other"
While:
stream2.join(stream1, ...) // stream2 will be materialized with "-this",
and stream1 with "-other"
If we think it is reasonable to require users be aware that the above join
situations are not exactly the same, then the current naming is fine; if we
want them to be mutually reused (I'm not sure if this is a common case?)
then we probably need to consider something new?
Guozhang
On Tue, Feb 13, 2018 at 7:02 PM, Matthias Margush <
matthias.margush@gmail.com> wrote:
> Thanks for the reminder! I need to do some wordsmithing based on the
> feedback I’ve gotten. I’ll do that soon (hopefully).
> On Tue, Feb 13, 2018 at 1:45 PM Matthias J. Sax <ma...@confluent.io>
> wrote:
>
> > Is there any updates for this KIP?
> >
> > -Matthias
> >
> > On 12/28/17 12:27 PM, Matthias J. Sax wrote:
> > > Thanks for updating the KIP.
> > >
> > > The code-diff is a little hard to read. It's better to so something
> > > similar as in this KIP:
> > >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 245%3A+Use+Properties+instead+of+StreamsConfig+in+KafkaStreams+constructor
> > >
> > > (Just as an example. Maybe take a look as other KIPs, too.)
> > >
> > > Some side remarks:
> > > - please update the link to the DISCUSS thread
> > > - there are some typos: Kstream -> KStream; Topology Builder exception
> > > -> TopologyBuilderException
> > >
> > >
> > > You propose to add `otherValueSerde(final String joinName)` -- I guess
> > > the method name is a c&p error and method name must be updated?
> > >
> > > Changes to internal classes like `KStreamImpl` are not required in the
> > > KIP as those as implementation details. The KIP should focus on public
> > > changes.
> > >
> > >
> > > -Matthias
> > >
> > >
> > > On 12/26/17 11:19 AM, Matthias Margush wrote:
> > >> Greetings.
> > >>
> > >> Thanks for the comments and suggestions. I updated the KIP with these
> > >> proposals for the questions posed by Matt & Matthias:
> > >>
> > >> *Can you please c&p the corresponding content instead of just
> > >> putting links? A KIP should be a self-contained Wiki page. Also, if we
> > add
> > >> a optional config parameter, how would we specify it? **Please list
> all
> > >> changes to want to apply to `Joined` class.*
> > >>
> > >> I added more details around the proposed changes directly to the KIP.
> > >>
> > >> *I will point out that your KIP doesn't outline what would happen if
> > >> you picked a name that resulted in a non unique topic name? What would
> > be
> > >> the error handling behavior there?*
> > >>
> > >> Looking at the current behavior of methods that allow the user to
> > specify
> > >> names for internal resources (e.g. `reduce`, `aggregate`), I added a
> > >> proposal that the code generate a similar exception if a name conflict
> > is
> > >> detected in the topology:
> > >>
> > >> org.apache.kafka.streams.errors.TopologyBuilderException: "Invalid
> > topology
> > >> building: Topic reduction-same-name-repartition has already been
> > registered
> > >> by another source."
> > >>
> > >> *What is the impact on KStream-KTable join?*
> > >>
> > >> Proposed that kstream-ktable joins similarly make use of the provided
> > >> joinName when generating internal repartition topics.
> > >>
> > >> On Mon, Dec 4, 2017 at 2:57 PM Matthias J. Sax <matthias@confluent.io
> >
> > >> wrote:
> > >>
> > >>> Matthias,
> > >>>
> > >>> thanks for the KIP.
> > >>>
> > >>> Can you please c&p the corresponding content instead of just putting
> > >>> links? A KIP should be a self-contained Wiki page.
> > >>>
> > >>> Also, if we add a optional config parameter, how would we specify it?
> > >>> Please list all changes to want to apply to `Joined` class.
> > >>>
> > >>> Furthermore, `Joined` is also used for KStream-KTable join but the
> KIP
> > >>> only talks about windowed joins (ie, KStream-KTream join). What the
> > >>> impact on KStream-KTable join?
> > >>>
> > >>>
> > >>> -Matthias
> > >>>
> > >>> On 11/29/17 6:09 AM, Matt Farmer wrote:
> > >>>> Hi Matthias,
> > >>>>
> > >>>> I certainly have found the auto-generated names unwieldy while doing
> > >>>> cluster administration.
> > >>>>
> > >>>> I will point out that your KIP doesn't outline what would happen if
> > you
> > >>>> picked a name that resulted in a non unique topic name? What would
> be
> > the
> > >>>> error handling behavior there?
> > >>>>
> > >>>> On Wed, Nov 29, 2017 at 9:03 AM Matthias Margush <
> > >>> matthias.margush@gmail.com>
> > >>>> wrote:
> > >>>>
> > >>>>> Hi everyone,
> > >>>>>
> > >>>>> I created this KIP to allow windowing joins to be named. If named,
> > then
> > >>> the
> > >>>>> associated internal topic names would be derived from that, instead
> > of
> > >>>>> being randomly generated.
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP+
> 230%3A+Name+Windowing+Joins
> > >>>>>
> > >>>>> Thanks,
> > >>>>>
> > >>>>> Matthias
> > >>>>>
> > >>>>
> > >>>
> > >>>
> > >>
> > >
> >
> >
>
--
-- Guozhang
Re: [DISCUSS]: KIP-230: Name Windowing Joins
Posted by Matthias Margush <ma...@gmail.com>.
Thanks for the reminder! I need to do some wordsmithing based on the
feedback I’ve gotten. I’ll do that soon (hopefully).
On Tue, Feb 13, 2018 at 1:45 PM Matthias J. Sax <ma...@confluent.io>
wrote:
> Is there any updates for this KIP?
>
> -Matthias
>
> On 12/28/17 12:27 PM, Matthias J. Sax wrote:
> > Thanks for updating the KIP.
> >
> > The code-diff is a little hard to read. It's better to so something
> > similar as in this KIP:
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-245%3A+Use+Properties+instead+of+StreamsConfig+in+KafkaStreams+constructor
> >
> > (Just as an example. Maybe take a look as other KIPs, too.)
> >
> > Some side remarks:
> > - please update the link to the DISCUSS thread
> > - there are some typos: Kstream -> KStream; Topology Builder exception
> > -> TopologyBuilderException
> >
> >
> > You propose to add `otherValueSerde(final String joinName)` -- I guess
> > the method name is a c&p error and method name must be updated?
> >
> > Changes to internal classes like `KStreamImpl` are not required in the
> > KIP as those as implementation details. The KIP should focus on public
> > changes.
> >
> >
> > -Matthias
> >
> >
> > On 12/26/17 11:19 AM, Matthias Margush wrote:
> >> Greetings.
> >>
> >> Thanks for the comments and suggestions. I updated the KIP with these
> >> proposals for the questions posed by Matt & Matthias:
> >>
> >> *Can you please c&p the corresponding content instead of just
> >> putting links? A KIP should be a self-contained Wiki page. Also, if we
> add
> >> a optional config parameter, how would we specify it? **Please list all
> >> changes to want to apply to `Joined` class.*
> >>
> >> I added more details around the proposed changes directly to the KIP.
> >>
> >> *I will point out that your KIP doesn't outline what would happen if
> >> you picked a name that resulted in a non unique topic name? What would
> be
> >> the error handling behavior there?*
> >>
> >> Looking at the current behavior of methods that allow the user to
> specify
> >> names for internal resources (e.g. `reduce`, `aggregate`), I added a
> >> proposal that the code generate a similar exception if a name conflict
> is
> >> detected in the topology:
> >>
> >> org.apache.kafka.streams.errors.TopologyBuilderException: "Invalid
> topology
> >> building: Topic reduction-same-name-repartition has already been
> registered
> >> by another source."
> >>
> >> *What is the impact on KStream-KTable join?*
> >>
> >> Proposed that kstream-ktable joins similarly make use of the provided
> >> joinName when generating internal repartition topics.
> >>
> >> On Mon, Dec 4, 2017 at 2:57 PM Matthias J. Sax <ma...@confluent.io>
> >> wrote:
> >>
> >>> Matthias,
> >>>
> >>> thanks for the KIP.
> >>>
> >>> Can you please c&p the corresponding content instead of just putting
> >>> links? A KIP should be a self-contained Wiki page.
> >>>
> >>> Also, if we add a optional config parameter, how would we specify it?
> >>> Please list all changes to want to apply to `Joined` class.
> >>>
> >>> Furthermore, `Joined` is also used for KStream-KTable join but the KIP
> >>> only talks about windowed joins (ie, KStream-KTream join). What the
> >>> impact on KStream-KTable join?
> >>>
> >>>
> >>> -Matthias
> >>>
> >>> On 11/29/17 6:09 AM, Matt Farmer wrote:
> >>>> Hi Matthias,
> >>>>
> >>>> I certainly have found the auto-generated names unwieldy while doing
> >>>> cluster administration.
> >>>>
> >>>> I will point out that your KIP doesn't outline what would happen if
> you
> >>>> picked a name that resulted in a non unique topic name? What would be
> the
> >>>> error handling behavior there?
> >>>>
> >>>> On Wed, Nov 29, 2017 at 9:03 AM Matthias Margush <
> >>> matthias.margush@gmail.com>
> >>>> wrote:
> >>>>
> >>>>> Hi everyone,
> >>>>>
> >>>>> I created this KIP to allow windowing joins to be named. If named,
> then
> >>> the
> >>>>> associated internal topic names would be derived from that, instead
> of
> >>>>> being randomly generated.
> >>>>>
> >>>>>
> >>>>>
> >>>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP+230%3A+Name+Windowing+Joins
> >>>>>
> >>>>> Thanks,
> >>>>>
> >>>>> Matthias
> >>>>>
> >>>>
> >>>
> >>>
> >>
> >
>
>
Re: [DISCUSS]: KIP-230: Name Windowing Joins
Posted by "Matthias J. Sax" <ma...@confluent.io>.
Is there any updates for this KIP?
-Matthias
On 12/28/17 12:27 PM, Matthias J. Sax wrote:
> Thanks for updating the KIP.
>
> The code-diff is a little hard to read. It's better to so something
> similar as in this KIP:
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-245%3A+Use+Properties+instead+of+StreamsConfig+in+KafkaStreams+constructor
>
> (Just as an example. Maybe take a look as other KIPs, too.)
>
> Some side remarks:
> - please update the link to the DISCUSS thread
> - there are some typos: Kstream -> KStream; Topology Builder exception
> -> TopologyBuilderException
>
>
> You propose to add `otherValueSerde(final String joinName)` -- I guess
> the method name is a c&p error and method name must be updated?
>
> Changes to internal classes like `KStreamImpl` are not required in the
> KIP as those as implementation details. The KIP should focus on public
> changes.
>
>
> -Matthias
>
>
> On 12/26/17 11:19 AM, Matthias Margush wrote:
>> Greetings.
>>
>> Thanks for the comments and suggestions. I updated the KIP with these
>> proposals for the questions posed by Matt & Matthias:
>>
>> *Can you please c&p the corresponding content instead of just
>> putting links? A KIP should be a self-contained Wiki page. Also, if we add
>> a optional config parameter, how would we specify it? **Please list all
>> changes to want to apply to `Joined` class.*
>>
>> I added more details around the proposed changes directly to the KIP.
>>
>> *I will point out that your KIP doesn't outline what would happen if
>> you picked a name that resulted in a non unique topic name? What would be
>> the error handling behavior there?*
>>
>> Looking at the current behavior of methods that allow the user to specify
>> names for internal resources (e.g. `reduce`, `aggregate`), I added a
>> proposal that the code generate a similar exception if a name conflict is
>> detected in the topology:
>>
>> org.apache.kafka.streams.errors.TopologyBuilderException: "Invalid topology
>> building: Topic reduction-same-name-repartition has already been registered
>> by another source."
>>
>> *What is the impact on KStream-KTable join?*
>>
>> Proposed that kstream-ktable joins similarly make use of the provided
>> joinName when generating internal repartition topics.
>>
>> On Mon, Dec 4, 2017 at 2:57 PM Matthias J. Sax <ma...@confluent.io>
>> wrote:
>>
>>> Matthias,
>>>
>>> thanks for the KIP.
>>>
>>> Can you please c&p the corresponding content instead of just putting
>>> links? A KIP should be a self-contained Wiki page.
>>>
>>> Also, if we add a optional config parameter, how would we specify it?
>>> Please list all changes to want to apply to `Joined` class.
>>>
>>> Furthermore, `Joined` is also used for KStream-KTable join but the KIP
>>> only talks about windowed joins (ie, KStream-KTream join). What the
>>> impact on KStream-KTable join?
>>>
>>>
>>> -Matthias
>>>
>>> On 11/29/17 6:09 AM, Matt Farmer wrote:
>>>> Hi Matthias,
>>>>
>>>> I certainly have found the auto-generated names unwieldy while doing
>>>> cluster administration.
>>>>
>>>> I will point out that your KIP doesn't outline what would happen if you
>>>> picked a name that resulted in a non unique topic name? What would be the
>>>> error handling behavior there?
>>>>
>>>> On Wed, Nov 29, 2017 at 9:03 AM Matthias Margush <
>>> matthias.margush@gmail.com>
>>>> wrote:
>>>>
>>>>> Hi everyone,
>>>>>
>>>>> I created this KIP to allow windowing joins to be named. If named, then
>>> the
>>>>> associated internal topic names would be derived from that, instead of
>>>>> being randomly generated.
>>>>>
>>>>>
>>>>>
>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+230%3A+Name+Windowing+Joins
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Matthias
>>>>>
>>>>
>>>
>>>
>>
>
Re: [DISCUSS]: KIP-230: Name Windowing Joins
Posted by "Matthias J. Sax" <ma...@confluent.io>.
Thanks for updating the KIP.
The code-diff is a little hard to read. It's better to so something
similar as in this KIP:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-245%3A+Use+Properties+instead+of+StreamsConfig+in+KafkaStreams+constructor
(Just as an example. Maybe take a look as other KIPs, too.)
Some side remarks:
- please update the link to the DISCUSS thread
- there are some typos: Kstream -> KStream; Topology Builder exception
-> TopologyBuilderException
You propose to add `otherValueSerde(final String joinName)` -- I guess
the method name is a c&p error and method name must be updated?
Changes to internal classes like `KStreamImpl` are not required in the
KIP as those as implementation details. The KIP should focus on public
changes.
-Matthias
On 12/26/17 11:19 AM, Matthias Margush wrote:
> Greetings.
>
> Thanks for the comments and suggestions. I updated the KIP with these
> proposals for the questions posed by Matt & Matthias:
>
> *Can you please c&p the corresponding content instead of just
> putting links? A KIP should be a self-contained Wiki page. Also, if we add
> a optional config parameter, how would we specify it? **Please list all
> changes to want to apply to `Joined` class.*
>
> I added more details around the proposed changes directly to the KIP.
>
> *I will point out that your KIP doesn't outline what would happen if
> you picked a name that resulted in a non unique topic name? What would be
> the error handling behavior there?*
>
> Looking at the current behavior of methods that allow the user to specify
> names for internal resources (e.g. `reduce`, `aggregate`), I added a
> proposal that the code generate a similar exception if a name conflict is
> detected in the topology:
>
> org.apache.kafka.streams.errors.TopologyBuilderException: "Invalid topology
> building: Topic reduction-same-name-repartition has already been registered
> by another source."
>
> *What is the impact on KStream-KTable join?*
>
> Proposed that kstream-ktable joins similarly make use of the provided
> joinName when generating internal repartition topics.
>
> On Mon, Dec 4, 2017 at 2:57 PM Matthias J. Sax <ma...@confluent.io>
> wrote:
>
>> Matthias,
>>
>> thanks for the KIP.
>>
>> Can you please c&p the corresponding content instead of just putting
>> links? A KIP should be a self-contained Wiki page.
>>
>> Also, if we add a optional config parameter, how would we specify it?
>> Please list all changes to want to apply to `Joined` class.
>>
>> Furthermore, `Joined` is also used for KStream-KTable join but the KIP
>> only talks about windowed joins (ie, KStream-KTream join). What the
>> impact on KStream-KTable join?
>>
>>
>> -Matthias
>>
>> On 11/29/17 6:09 AM, Matt Farmer wrote:
>>> Hi Matthias,
>>>
>>> I certainly have found the auto-generated names unwieldy while doing
>>> cluster administration.
>>>
>>> I will point out that your KIP doesn't outline what would happen if you
>>> picked a name that resulted in a non unique topic name? What would be the
>>> error handling behavior there?
>>>
>>> On Wed, Nov 29, 2017 at 9:03 AM Matthias Margush <
>> matthias.margush@gmail.com>
>>> wrote:
>>>
>>>> Hi everyone,
>>>>
>>>> I created this KIP to allow windowing joins to be named. If named, then
>> the
>>>> associated internal topic names would be derived from that, instead of
>>>> being randomly generated.
>>>>
>>>>
>>>>
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+230%3A+Name+Windowing+Joins
>>>>
>>>> Thanks,
>>>>
>>>> Matthias
>>>>
>>>
>>
>>
>
Re: [DISCUSS]: KIP-230: Name Windowing Joins
Posted by Matthias Margush <ma...@gmail.com>.
Greetings.
Thanks for the comments and suggestions. I updated the KIP with these
proposals for the questions posed by Matt & Matthias:
*Can you please c&p the corresponding content instead of just
putting links? A KIP should be a self-contained Wiki page. Also, if we add
a optional config parameter, how would we specify it? **Please list all
changes to want to apply to `Joined` class.*
I added more details around the proposed changes directly to the KIP.
*I will point out that your KIP doesn't outline what would happen if
you picked a name that resulted in a non unique topic name? What would be
the error handling behavior there?*
Looking at the current behavior of methods that allow the user to specify
names for internal resources (e.g. `reduce`, `aggregate`), I added a
proposal that the code generate a similar exception if a name conflict is
detected in the topology:
org.apache.kafka.streams.errors.TopologyBuilderException: "Invalid topology
building: Topic reduction-same-name-repartition has already been registered
by another source."
*What is the impact on KStream-KTable join?*
Proposed that kstream-ktable joins similarly make use of the provided
joinName when generating internal repartition topics.
On Mon, Dec 4, 2017 at 2:57 PM Matthias J. Sax <ma...@confluent.io>
wrote:
> Matthias,
>
> thanks for the KIP.
>
> Can you please c&p the corresponding content instead of just putting
> links? A KIP should be a self-contained Wiki page.
>
> Also, if we add a optional config parameter, how would we specify it?
> Please list all changes to want to apply to `Joined` class.
>
> Furthermore, `Joined` is also used for KStream-KTable join but the KIP
> only talks about windowed joins (ie, KStream-KTream join). What the
> impact on KStream-KTable join?
>
>
> -Matthias
>
> On 11/29/17 6:09 AM, Matt Farmer wrote:
> > Hi Matthias,
> >
> > I certainly have found the auto-generated names unwieldy while doing
> > cluster administration.
> >
> > I will point out that your KIP doesn't outline what would happen if you
> > picked a name that resulted in a non unique topic name? What would be the
> > error handling behavior there?
> >
> > On Wed, Nov 29, 2017 at 9:03 AM Matthias Margush <
> matthias.margush@gmail.com>
> > wrote:
> >
> >> Hi everyone,
> >>
> >> I created this KIP to allow windowing joins to be named. If named, then
> the
> >> associated internal topic names would be derived from that, instead of
> >> being randomly generated.
> >>
> >>
> >>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP+230%3A+Name+Windowing+Joins
> >>
> >> Thanks,
> >>
> >> Matthias
> >>
> >
>
>