You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by omkar mestry <om...@gmail.com> on 2019/05/26 06:18:02 UTC

[DISCUSS] KIP-474: To deprecate WindowStore#put(key, value)

We propose to deprecate the WindowStore#put(key, value), as it does not
have a timestamp as a parameter. The window store requires a timestamp to
map the key to a window frame. This method uses the current record
timestamp(as specified in the description of the method). There is a method
present with a timestamp as a parameter which can be used instead.

Re: [DISCUSS] KIP-474: To deprecate WindowStore#put(key, value)

Posted by Guozhang Wang <wa...@gmail.com>.
I've made a pass on the KIP and it looks good to me as well. I think we can
start a voting thread for it.

On Thu, May 30, 2019 at 10:27 PM Matthias J. Sax <ma...@confluent.io>
wrote:

> Thanks for the KIP Omkar.
>
> I think this is rather uncontroversial, and I also support this KIP. I
> think you can start a VOTE. People can still chime in on the discuss
> thread if they have any concerns.
>
>
> -Matthias
>
> On 5/27/19 11:50 PM, Dongjin Lee wrote:
> > Hi Omkar,
> >
> > Looks good to me. Thanks!
> >
> > Is there anyone who has some comments about the KIP?
> >
> > Thanks,
> > Dongjin
> >
> > On Tue, May 28, 2019 at 3:24 PM omkar mestry <om...@gmail.com>
> wrote:
> >
> >> Hi Dongjin,
> >>
> >> I have updated the KIP please have a look and provide feedback on it.
> >>
> >> KIP :-
> >>
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=115526545
> >>
> >> Thanks & Regards
> >> Omkar Mestry
> >>
> >> On Mon, May 27, 2019 at 6:25 PM Dongjin Lee <do...@apache.org> wrote:
> >>
> >>> Hi Omkar,
> >>>
> >>> Thanks for the KIP. However, discussion thread should include a link to
> >> the
> >>> KIP document. Since you omitted it, here is the link.
> >>>
> >>>
> >>
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=115526545
> >>>
> >>> As far as I understand, the point of the KIP is the current API can
> >> result
> >>> in inconsistency to should be deprecated and finally, removed. Right?
> >> Here
> >>> are some comments on the KIP.
> >>>
> >>> *1. Minor corrections*
> >>>
> >>> Since the KIP proposes deprecation of an API, not actually removing it,
> >> it
> >>> would be better to correct the following sentences:
> >>>
> >>> - "Therefore by removing the method put(key, value), we can prevent
> >>> inconsistency." → "Therefore by deprecating (and finally removing) the
> >>> method put(key, value), we can prevent inconsistency."
> >>> - "Also, there are tests which are needed to be updated after removal
> of
> >>> the specified method." → "Also, there are tests which are needed to be
> >>> updated after deprecation of the specified method."
> >>>
> >>> *2. About 'Motivation' section*
> >>>
> >>> I think the motivation section can be more clear by referring to the
> risk
> >>> of the current API. How do you think?
> >>>
> >>> "... Therefore by ..."
> >>>
> >>> → "... This constraint makes WindowStore error prone. Therefore by ..."
> >>>
> >>> *3. About 'Rejected Alternatives' section*
> >>>
> >>> This sections should state why these alternatives were rejected. How
> >> about
> >>> this?
> >>>
> >>> "Since this API can be called by the user[^1][^2], updating the method
> >> can
> >>> break the code; By this reason, this approach is not feasible."
> >>>
> >>> Regards,
> >>> Dongjin
> >>>
> >>> [^1]:
> >>>
> >>>
> >>
> https://kafka.apache.org/22/javadoc/org/apache/kafka/streams/state/Stores.html
> >>> [^2]:
> >>>
> >>>
> >>
> https://kafka.apache.org/22/javadoc/org/apache/kafka/streams/state/WindowStore.html
> >>>
> >>> On Sun, May 26, 2019 at 3:18 PM omkar mestry <om...@gmail.com>
> >>> wrote:
> >>>
> >>>> We propose to deprecate the WindowStore#put(key, value), as it does
> not
> >>>> have a timestamp as a parameter. The window store requires a timestamp
> >> to
> >>>> map the key to a window frame. This method uses the current record
> >>>> timestamp(as specified in the description of the method). There is a
> >>> method
> >>>> present with a timestamp as a parameter which can be used instead.
> >>>>
> >>>
> >>>
> >>> --
> >>> *Dongjin Lee*
> >>>
> >>> *A hitchhiker in the mathematical world.*
> >>> *github:  <http://goog_969573159/>github.com/dongjinleekr
> >>> <https://github.com/dongjinleekr>linkedin:
> >> kr.linkedin.com/in/dongjinleekr
> >>> <https://kr.linkedin.com/in/dongjinleekr>speakerdeck:
> >>> speakerdeck.com/dongjin
> >>> <https://speakerdeck.com/dongjin>*
> >>>
> >>
> >
> >
>
>

-- 
-- Guozhang

Re: [DISCUSS] KIP-474: To deprecate WindowStore#put(key, value)

Posted by "Matthias J. Sax" <ma...@confluent.io>.
Thanks for the KIP Omkar.

I think this is rather uncontroversial, and I also support this KIP. I
think you can start a VOTE. People can still chime in on the discuss
thread if they have any concerns.


-Matthias

On 5/27/19 11:50 PM, Dongjin Lee wrote:
> Hi Omkar,
> 
> Looks good to me. Thanks!
> 
> Is there anyone who has some comments about the KIP?
> 
> Thanks,
> Dongjin
> 
> On Tue, May 28, 2019 at 3:24 PM omkar mestry <om...@gmail.com> wrote:
> 
>> Hi Dongjin,
>>
>> I have updated the KIP please have a look and provide feedback on it.
>>
>> KIP :-
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=115526545
>>
>> Thanks & Regards
>> Omkar Mestry
>>
>> On Mon, May 27, 2019 at 6:25 PM Dongjin Lee <do...@apache.org> wrote:
>>
>>> Hi Omkar,
>>>
>>> Thanks for the KIP. However, discussion thread should include a link to
>> the
>>> KIP document. Since you omitted it, here is the link.
>>>
>>>
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=115526545
>>>
>>> As far as I understand, the point of the KIP is the current API can
>> result
>>> in inconsistency to should be deprecated and finally, removed. Right?
>> Here
>>> are some comments on the KIP.
>>>
>>> *1. Minor corrections*
>>>
>>> Since the KIP proposes deprecation of an API, not actually removing it,
>> it
>>> would be better to correct the following sentences:
>>>
>>> - "Therefore by removing the method put(key, value), we can prevent
>>> inconsistency." → "Therefore by deprecating (and finally removing) the
>>> method put(key, value), we can prevent inconsistency."
>>> - "Also, there are tests which are needed to be updated after removal of
>>> the specified method." → "Also, there are tests which are needed to be
>>> updated after deprecation of the specified method."
>>>
>>> *2. About 'Motivation' section*
>>>
>>> I think the motivation section can be more clear by referring to the risk
>>> of the current API. How do you think?
>>>
>>> "... Therefore by ..."
>>>
>>> → "... This constraint makes WindowStore error prone. Therefore by ..."
>>>
>>> *3. About 'Rejected Alternatives' section*
>>>
>>> This sections should state why these alternatives were rejected. How
>> about
>>> this?
>>>
>>> "Since this API can be called by the user[^1][^2], updating the method
>> can
>>> break the code; By this reason, this approach is not feasible."
>>>
>>> Regards,
>>> Dongjin
>>>
>>> [^1]:
>>>
>>>
>> https://kafka.apache.org/22/javadoc/org/apache/kafka/streams/state/Stores.html
>>> [^2]:
>>>
>>>
>> https://kafka.apache.org/22/javadoc/org/apache/kafka/streams/state/WindowStore.html
>>>
>>> On Sun, May 26, 2019 at 3:18 PM omkar mestry <om...@gmail.com>
>>> wrote:
>>>
>>>> We propose to deprecate the WindowStore#put(key, value), as it does not
>>>> have a timestamp as a parameter. The window store requires a timestamp
>> to
>>>> map the key to a window frame. This method uses the current record
>>>> timestamp(as specified in the description of the method). There is a
>>> method
>>>> present with a timestamp as a parameter which can be used instead.
>>>>
>>>
>>>
>>> --
>>> *Dongjin Lee*
>>>
>>> *A hitchhiker in the mathematical world.*
>>> *github:  <http://goog_969573159/>github.com/dongjinleekr
>>> <https://github.com/dongjinleekr>linkedin:
>> kr.linkedin.com/in/dongjinleekr
>>> <https://kr.linkedin.com/in/dongjinleekr>speakerdeck:
>>> speakerdeck.com/dongjin
>>> <https://speakerdeck.com/dongjin>*
>>>
>>
> 
> 


Re: [DISCUSS] KIP-474: To deprecate WindowStore#put(key, value)

Posted by Dongjin Lee <do...@apache.org>.
Hi Omkar,

Looks good to me. Thanks!

Is there anyone who has some comments about the KIP?

Thanks,
Dongjin

On Tue, May 28, 2019 at 3:24 PM omkar mestry <om...@gmail.com> wrote:

> Hi Dongjin,
>
> I have updated the KIP please have a look and provide feedback on it.
>
> KIP :-
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=115526545
>
> Thanks & Regards
> Omkar Mestry
>
> On Mon, May 27, 2019 at 6:25 PM Dongjin Lee <do...@apache.org> wrote:
>
> > Hi Omkar,
> >
> > Thanks for the KIP. However, discussion thread should include a link to
> the
> > KIP document. Since you omitted it, here is the link.
> >
> >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=115526545
> >
> > As far as I understand, the point of the KIP is the current API can
> result
> > in inconsistency to should be deprecated and finally, removed. Right?
> Here
> > are some comments on the KIP.
> >
> > *1. Minor corrections*
> >
> > Since the KIP proposes deprecation of an API, not actually removing it,
> it
> > would be better to correct the following sentences:
> >
> > - "Therefore by removing the method put(key, value), we can prevent
> > inconsistency." → "Therefore by deprecating (and finally removing) the
> > method put(key, value), we can prevent inconsistency."
> > - "Also, there are tests which are needed to be updated after removal of
> > the specified method." → "Also, there are tests which are needed to be
> > updated after deprecation of the specified method."
> >
> > *2. About 'Motivation' section*
> >
> > I think the motivation section can be more clear by referring to the risk
> > of the current API. How do you think?
> >
> > "... Therefore by ..."
> >
> > → "... This constraint makes WindowStore error prone. Therefore by ..."
> >
> > *3. About 'Rejected Alternatives' section*
> >
> > This sections should state why these alternatives were rejected. How
> about
> > this?
> >
> > "Since this API can be called by the user[^1][^2], updating the method
> can
> > break the code; By this reason, this approach is not feasible."
> >
> > Regards,
> > Dongjin
> >
> > [^1]:
> >
> >
> https://kafka.apache.org/22/javadoc/org/apache/kafka/streams/state/Stores.html
> > [^2]:
> >
> >
> https://kafka.apache.org/22/javadoc/org/apache/kafka/streams/state/WindowStore.html
> >
> > On Sun, May 26, 2019 at 3:18 PM omkar mestry <om...@gmail.com>
> > wrote:
> >
> > > We propose to deprecate the WindowStore#put(key, value), as it does not
> > > have a timestamp as a parameter. The window store requires a timestamp
> to
> > > map the key to a window frame. This method uses the current record
> > > timestamp(as specified in the description of the method). There is a
> > method
> > > present with a timestamp as a parameter which can be used instead.
> > >
> >
> >
> > --
> > *Dongjin Lee*
> >
> > *A hitchhiker in the mathematical world.*
> > *github:  <http://goog_969573159/>github.com/dongjinleekr
> > <https://github.com/dongjinleekr>linkedin:
> kr.linkedin.com/in/dongjinleekr
> > <https://kr.linkedin.com/in/dongjinleekr>speakerdeck:
> > speakerdeck.com/dongjin
> > <https://speakerdeck.com/dongjin>*
> >
>


-- 
*Dongjin Lee*

*A hitchhiker in the mathematical world.*
*github:  <http://goog_969573159/>github.com/dongjinleekr
<https://github.com/dongjinleekr>linkedin: kr.linkedin.com/in/dongjinleekr
<https://kr.linkedin.com/in/dongjinleekr>speakerdeck: speakerdeck.com/dongjin
<https://speakerdeck.com/dongjin>*

Re: [DISCUSS] KIP-474: To deprecate WindowStore#put(key, value)

Posted by omkar mestry <om...@gmail.com>.
Hi Dongjin,

I have updated the KIP please have a look and provide feedback on it.

KIP :-
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=115526545

Thanks & Regards
Omkar Mestry

On Mon, May 27, 2019 at 6:25 PM Dongjin Lee <do...@apache.org> wrote:

> Hi Omkar,
>
> Thanks for the KIP. However, discussion thread should include a link to the
> KIP document. Since you omitted it, here is the link.
>
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=115526545
>
> As far as I understand, the point of the KIP is the current API can result
> in inconsistency to should be deprecated and finally, removed. Right? Here
> are some comments on the KIP.
>
> *1. Minor corrections*
>
> Since the KIP proposes deprecation of an API, not actually removing it, it
> would be better to correct the following sentences:
>
> - "Therefore by removing the method put(key, value), we can prevent
> inconsistency." → "Therefore by deprecating (and finally removing) the
> method put(key, value), we can prevent inconsistency."
> - "Also, there are tests which are needed to be updated after removal of
> the specified method." → "Also, there are tests which are needed to be
> updated after deprecation of the specified method."
>
> *2. About 'Motivation' section*
>
> I think the motivation section can be more clear by referring to the risk
> of the current API. How do you think?
>
> "... Therefore by ..."
>
> → "... This constraint makes WindowStore error prone. Therefore by ..."
>
> *3. About 'Rejected Alternatives' section*
>
> This sections should state why these alternatives were rejected. How about
> this?
>
> "Since this API can be called by the user[^1][^2], updating the method can
> break the code; By this reason, this approach is not feasible."
>
> Regards,
> Dongjin
>
> [^1]:
>
> https://kafka.apache.org/22/javadoc/org/apache/kafka/streams/state/Stores.html
> [^2]:
>
> https://kafka.apache.org/22/javadoc/org/apache/kafka/streams/state/WindowStore.html
>
> On Sun, May 26, 2019 at 3:18 PM omkar mestry <om...@gmail.com>
> wrote:
>
> > We propose to deprecate the WindowStore#put(key, value), as it does not
> > have a timestamp as a parameter. The window store requires a timestamp to
> > map the key to a window frame. This method uses the current record
> > timestamp(as specified in the description of the method). There is a
> method
> > present with a timestamp as a parameter which can be used instead.
> >
>
>
> --
> *Dongjin Lee*
>
> *A hitchhiker in the mathematical world.*
> *github:  <http://goog_969573159/>github.com/dongjinleekr
> <https://github.com/dongjinleekr>linkedin: kr.linkedin.com/in/dongjinleekr
> <https://kr.linkedin.com/in/dongjinleekr>speakerdeck:
> speakerdeck.com/dongjin
> <https://speakerdeck.com/dongjin>*
>

Re: [DISCUSS] KIP-474: To deprecate WindowStore#put(key, value)

Posted by Dongjin Lee <do...@apache.org>.
Hi Omkar,

Thanks for the KIP. However, discussion thread should include a link to the
KIP document. Since you omitted it, here is the link.

https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=115526545

As far as I understand, the point of the KIP is the current API can result
in inconsistency to should be deprecated and finally, removed. Right? Here
are some comments on the KIP.

*1. Minor corrections*

Since the KIP proposes deprecation of an API, not actually removing it, it
would be better to correct the following sentences:

- "Therefore by removing the method put(key, value), we can prevent
inconsistency." → "Therefore by deprecating (and finally removing) the
method put(key, value), we can prevent inconsistency."
- "Also, there are tests which are needed to be updated after removal of
the specified method." → "Also, there are tests which are needed to be
updated after deprecation of the specified method."

*2. About 'Motivation' section*

I think the motivation section can be more clear by referring to the risk
of the current API. How do you think?

"... Therefore by ..."

→ "... This constraint makes WindowStore error prone. Therefore by ..."

*3. About 'Rejected Alternatives' section*

This sections should state why these alternatives were rejected. How about
this?

"Since this API can be called by the user[^1][^2], updating the method can
break the code; By this reason, this approach is not feasible."

Regards,
Dongjin

[^1]:
https://kafka.apache.org/22/javadoc/org/apache/kafka/streams/state/Stores.html
[^2]:
https://kafka.apache.org/22/javadoc/org/apache/kafka/streams/state/WindowStore.html

On Sun, May 26, 2019 at 3:18 PM omkar mestry <om...@gmail.com> wrote:

> We propose to deprecate the WindowStore#put(key, value), as it does not
> have a timestamp as a parameter. The window store requires a timestamp to
> map the key to a window frame. This method uses the current record
> timestamp(as specified in the description of the method). There is a method
> present with a timestamp as a parameter which can be used instead.
>


-- 
*Dongjin Lee*

*A hitchhiker in the mathematical world.*
*github:  <http://goog_969573159/>github.com/dongjinleekr
<https://github.com/dongjinleekr>linkedin: kr.linkedin.com/in/dongjinleekr
<https://kr.linkedin.com/in/dongjinleekr>speakerdeck: speakerdeck.com/dongjin
<https://speakerdeck.com/dongjin>*