A comment on the PR pointed out the advantages of a relatively minor adjustment
to the method org.apache.kafka.streams.processor.addGlobalStore. It should take
a StateStoreSupplier" type as the first argument, not a
StateStore. Note that one can obtain a StateStore by calling the .get() method
Made a slight change to the wiki since I had forgotten to state the fact that
some APIs don’t take a store name, but a StateStoreSupplier (that contains the
name). That’s already the case in the code but thought of making that explicit.
FYI, no action needed.
Thanks
Eno
> On Apr 21, 2017, at 7:
Added the .tostream() discussion in the Wiki.
Thanks
Eno
> On 21 Apr 2017, at 18:48, Matthias J. Sax wrote:
>
> I agree with Eno about the renaming.
>
> @Eno: can you add this to the Wiki Discussion page?
>
>
> -Matthias
>
>
> On 4/21/17 1:11 AM, Eno Thereska wrote:
>> Hi Guozhang,
>>
>> T
Sounds good. Thanks.
On Fri, Apr 21, 2017 at 10:48 AM, Matthias J. Sax
wrote:
> I agree with Eno about the renaming.
>
> @Eno: can you add this to the Wiki Discussion page?
>
>
> -Matthias
>
>
> On 4/21/17 1:11 AM, Eno Thereska wrote:
> > Hi Guozhang,
> >
> > Thanks for the feedback. Comments in
I agree with Eno about the renaming.
@Eno: can you add this to the Wiki Discussion page?
-Matthias
On 4/21/17 1:11 AM, Eno Thereska wrote:
> Hi Guozhang,
>
> Thanks for the feedback. Comments inline:
>
>> 1. Regarding the user-facing semantics, I thought we will claim that
>> "KTables genera
t;>>>>>>>>>>>> decoupled from the DSL.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> We could think of ways to get store handles from part
>>
t on IQ:
> >>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>> 1. The first issue of the current DSL is that, there is
> >>>>>>>>>>>>>>>>>>&
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> 1. The first issue of the current DSL is that, there is
>>>>>>>>>>>>>>>>>>> inconsistency upon
>>>>>>
; e.g. KTables involved in joins; a RTE will be thrown not
>>>>>>>>>>>>>>>>>> immediately but
>>>>>>>>>>>>>>>>>> later in this case.
>>>>>>>>>>>>>>>&
;>>>>>>>> (dummy?) state
>>>>>>>>>>>>>>>>> store name for them; while on the other hand users cannot
>>>> query
>>>>>>>>>>>>>>>>> some state
>>>>>>>>>>>>&g
;>>>> For
>>>>>>>>>>>>>>>> serdes
>>>>>>>>>>>>>>>> specifically, we had a very long discussion about it and
>>>>>>> concluded
>>>>>>>>>>>>>>>> that, at
&g
t;>>>>>>>>>>>
>>>>>>>>>>>>>>> So to me, for either materialize() v.s. overloaded functions
>>>>>>>>>>>>>>> directions,
>>>>>>>>>>>>>>> the first
;>>>>>>>>>>>>> directions,
>>>>>>>>>>>>>>> the first thing I'd like to resolve is the inconsistency
>> issue
>>>>>>>>>>>>>>> mentioned
>>>>>>>>
>>>>>>>>>>>> this
> >>>>>>>>>>>>> param it is null by default);
> >>>>>>>>>>>>> b) null-state-store-name do not indicate that a KTable would
> >>>>>>
gt;>>>>> b) KTables that not calling materialized do not indicate that
>>> a
>>>>>>>>>>>>> KTable
>>>>>>>>>>>>> would not be materialized, but that it will not be used for IQ
>>>> a
nd the scene, to e.g. Damian suggested
>> "queryableStore(String
>>>>>>>>>>>> storeName)",
>>>>>>>>>>>> which returns a QueryableStateStore, and can replace the
>>>>>>>>>>>> `
hings!
>> The
>>>>>>>>>>>> thread
>>>>>>>>>>>> unfortunately split but as all branches close in on what my
>>>>>>>>>>>> suggestion was
>>>>>>>>>>>> about Ill p
gt;>>>>>>>
> > >>>>>>>>>> Of course only the table the user wants to query would be
> > >>>>>>>>>> materialized.
> > >>>>>>>>>> (retrieving the queryhandle implies
ss the
> streams
> >>>>>>>>>> instance and then basically uses the same mechanism that is
> >>>>>>>>>> currently used.
> >>>>>>>>>> From my point of view this is the least confusi
t;>>>>>>> higher
>>>>>>>>>> than the overloaded materialized call. As long as I could help
>>>>>>>>>> getting a
>>>>>>>>>> different view I am happy.
>>>>>>>>>>
>>&
gt;>>> Materialising
>>>>>>>>>> every KTable can be expensive, although there are some tricks one
>>>>>>>>>> can play,
>>>>>>>>>> e.g., have a virtual store rather than one backed by a Kafka topic.
>>>>>>>>>>
e'll consider as part of this KIP.
>>>>>>>>>
>>>>>>>>> Thanks
>>>>>>>>> Eno
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 28
gt;>>>>> easily. (In
>>>>>>>>> my Stream applications I do this a lot with reflection and
>>>>>>>>> instanciating
>>>>>>>>> KTableImpl) Concerning this KIP all I say is that there should
>>>>>
cc from user list
Forwarded Message
Subject: Re: [DISCUSS] KIP-114: KTable materialization and improved
semantics
Date: Mon, 30 Jan 2017 09:23:04 -0800
From: Matthias J. Sax
Organization: Confluent Inc
To: us...@kafka.apache.org
Hi,
I think Eno's separation is very clea
now
>>>>>>>> the Store name), so we are working on different levels to achieve a
>>>>>>>> single goal.
>>>>>>>>
>>>>>>>> What is your peoples opinion on having a method on KTABLE that returns
>>>>>>>&
;>>>>>>
>>>>>>> On 1/27/17 11:10 AM, Jan Filipiak wrote:
>>>>>>>
>>>>>>>> Yeah,
>>>>>>>>
>>>>>>>> Maybe my bad that I refuse to look into IQ as i don't find
;>>>
>>>>>>
>>>>>>
>>>>>> On 27.01.2017 16:51, Damian Guy wrote:
>>>>>>
>>>>>>> I think Jan is saying that they don't always need to be materialized,
>>>>>>> i.e.,
>>>>>>> fil
ist
>
>
> Forwarded Message
> Subject: Re: [DISCUSS] KIP-114: KTable materialization and improved
> semantics
> Date: Mon, 30 Jan 2017 00:06:37 -0800
> From: Matthias J. Sax
> Organization: Confluent Inc
> To: us...@kafka.apache.org
>
> I understand point (1
cc from user list
Forwarded Message
Subject: Re: [DISCUSS] KIP-114: KTable materialization and improved
semantics
Date: Mon, 30 Jan 2017 00:06:37 -0800
From: Matthias J. Sax
Organization: Confluent Inc
To: us...@kafka.apache.org
I understand point (1) about when
>> On Fri, 27 Jan 2017 at 15:49 Michael Noll
>>>>>> wrote:
>>>>>>
>>>>>> Like Damian, and for the same reasons, I am more in favor of
>>>>>>> overloading
>>>>>>> methods rather than introducing `materialize()`.
>
; I cannot see the reason for the additional materialize method being
>>> required! Hence I suggest leave it alone.
>>> regarding removing the others I dont have strong opinions and it seems
to
>>> be unrelated.
>>>
>>> Best Jan
>>>
>>>
>
>>> regarding removing the others I dont have strong opinions and it seems to
>>> be unrelated.
>>>
>>> Best Jan
>>>
>>>
>>>
>>>
>>> On 26.01.2017 20:48, Eno Thereska wrote:
>>>
>>>> Forwarding th
;>>>>
>>>>> On Fri, Jan 27, 2017 at 4:38 PM, Jan Filipiak
>>>>> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Yeah its confusing, Why shoudn't it be querable by IQ? If you uses t
That not what I meant by "huge impact".
I refer to the actions related to materialize a KTable: creating a
RocksDB store and a changelog topic -- users should be aware about
runtime implication and this is better expressed by an explicit method
call, rather than implicitly triggered by using a dif
I think your definition of a huge impact and mine are rather different ;-P
Overloading a few methods is not really a huge impact IMO. It is also a
sacrifice worth making for readability, usability of the API.
On Mon, 23 Jan 2017 at 17:55 Matthias J. Sax wrote:
> I understand your argument, but
I understand your argument, but do not agree with it.
Your first version (even if the "flow" is not as nice) is more explicit
than the second version. Adding a stateStoreName parameter is quite
implicit but has a huge impact -- thus, I prefer the rather more verbose
but explicit version.
-Matthi
I'm not a fan of materialize. I think it interrupts the flow, i.e,
table.mapValue(..).materialize().join(..).materialize()
compared to:
table.mapValues(..).join(..)
I know which one i prefer.
My preference is stil to provide overloaded methods where people can
specify the store names if they want
Hi,
thanks for the KIP Eno! Here are my 2 cents:
1) I like Guozhang's proposal about removing store name from all KTable
methods and generate internal names (however, I would do this as
overloads). Furthermore, I would not force users to call .materialize()
if they want to query a store, but add
Thanks for the KIP Eno, I have a few meta comments and a few detailed
comments:
1. I like the materialize() function in general, but I would like to see
how other KTable functions should be updated accordingly. For example, 1)
KStreamBuilder.table(..) has a state store name parameter, and we will
I think changing it to `toKStream` would make it absolutely clear what we are
converting it to.
I'd say we should probably change the KStreamBuilder methods (but not in this
KIP).
Thanks
Eno
> On 17 Jan 2017, at 13:59, Michael Noll wrote:
>
>> Rename toStream() to toKStream() for consistency
Hi Michael,
Answers inline:
> On 17 Jan 2017, at 12:09, Michael Noll wrote:
>
> If a KTable models a changelog, why do we call it a KTable, rather than
> (say) KChangelogStream? (Perhaps the use of the word "log" in "changelog"
> invokes the concept of a "stream" more than the concept of a ta
Good point, thanks, will update.
Eno
> On 17 Jan 2017, at 12:04, Michael Noll wrote:
>
> I think section "Compatibility, Deprecation, and Migration Plan" needs
> updating. It currently reads "No impact on existing users" but the KIP
> proposes to remove existing API methods (such as `KTable#for
> Rename toStream() to toKStream() for consistency.
Not sure whether that is really required. We also use
`KStreamBuilder#stream()` and `KStreamBuilder#table()`, for example, and
don't care about the "K" prefix.
On Tue, Jan 17, 2017 at 10:55 AM, Eno Thereska
wrote:
> Thanks Damian, answers in
Thanks for the KIP, Eno.
In section "Rejected Alternatives" the KIP says:
> It is not clear that collapsing 2 abstractions helps. In particular, a
KTable models a changelog.
> That itself is a useful abstraction. A state store is a materialized
view. That’s a distinct abstraction
> with parallels
I think section "Compatibility, Deprecation, and Migration Plan" needs
updating. It currently reads "No impact on existing users" but the KIP
proposes to remove existing API methods (such as `KTable#foreach()`).
-Michael
On Tue, Jan 17, 2017 at 10:55 AM, Eno Thereska
wrote:
> Thanks Damian, a
Thanks Damian, answers inline:
> On 16 Jan 2017, at 17:17, Damian Guy wrote:
>
> Hi Eno,
>
> Thanks for the KIP. Some comments:
>
> 1. I'd probably rename materialized to materialize.
Ok.
> 2. I don't think the addition of the new Log compaction mechanism is
> necessary for this KIP, i
Hi Eno,
Thanks for the KIP. Some comments:
1. I'd probably rename materialized to materialize.
2. I don't think the addition of the new Log compaction mechanism is
necessary for this KIP, i.e, the KIP is useful without it. Maybe that
should be a different KIP?
3. What will happen w
That's a good idea. I'll keep KIP-114 for the streams changes only and do a new
KIP soon for the log compaction as both are independent improvements.
I'm removing 2 paragraphs from KIP-114, but otherwise it's still good to
discuss on this thread.
Thanks
Eno
> On 16 Jan 2017, at 17:12, Ismael Ju
Thanks for the KIP, Eno. It seems reasonable on first inspection although
others more familiar with Streams may have more to say. :) With regards to "log
compaction based on timestamps", it may make sense to do that as its own
KIP as interested parties may miss it if it's inside a Streams KIP. It
w
49 matches
Mail list logo
