kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthias J. Sax" <matth...@confluent.io>
Subject Re: [DISCUSS]: KIP-230: Name Windowing Joins
Date Thu, 28 Dec 2017 20:27:29 GMT
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
>>>>
>>>
>>
>>
> 


Mime
View raw message