Thanks for the KIP Sophie. Overall LGTM. One typo: "that have not yet that their iterator closed"
I also added the KIP to https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Streams -Matthias On 2/14/19 3:29 PM, Guozhang Wang wrote: > Made another pass over the KIP page, lgtm! > > On Thu, Feb 14, 2019 at 3:05 PM Sophie Blee-Goldman <sop...@confluent.io> > wrote: > >> Cleaned up the KIP, please take another look and if all seems good will >> call for a vote since there seem to be no strong opinions against. >> >> On Wed, Feb 13, 2019 at 11:45 PM Guozhang Wang <wangg...@gmail.com> wrote: >> >>> Hi Sophie, >>> >>> Thanks for the KIP write-up, I made a pass over the wiki and the PR as >>> well, here's some comments: >>> >>> 1. the proposed API seems to be inconsistent from the PR, should it be: >>> >>> public static WindowBytesStoreSupplier inMemoryWindowStore(final String >>> name, >>> >>> final Duration retentionPeriod, >>> >>> final Duration windowSize, >>> + >>> final boolean retainDuplicates) throws >>> IllegalArgumentException ... >>> - >>> final Duration gracePeriod >>> >>> 2. As Boyang mentioned, we usually do not need to elaborate on the >> internal >>> implementation in the KIP, unless it has some user-facing implications. >> As >>> for this specific KIP, I think it make more sense to talk about what >> memory >>> footprint users would expect to have with the implementation: should they >>> be expecting exact number of bytes used for key-value pairs only, or >> should >>> they expect some additional memory used for maintaining the window data >>> structures. >>> >>> >>> >>> Guozhang >>> >>> >>> >>> >>> On Fri, Feb 8, 2019 at 4:21 AM Dongjin Lee <dong...@apache.org> wrote: >>> >>>> Thanks for the KIP, Sophie. I added your KIP into the 'Under >> Discussion' >>>> section here >>>> < >>>> >>> >> https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals >>>>> >>>> . >>>> >>>> I am +1 for this proposal for reaching parity between key-value store >> and >>>> windowed store. >>>> >>>> Thanks, >>>> Dongjin >>>> >>>> On Fri, Feb 8, 2019 at 1:41 PM Boyang Chen <bche...@outlook.com> >> wrote: >>>> >>>>> Thanks Sophie for proposing this new feature! In-memory window store >> is >>>>> very useful in long term. One meta comment is that we don't need to >>>> include >>>>> implementation details in the public interface section, and those >>>>> validation steps are pretty trivial. >>>>> >>>>> Boyang >>>>> >>>>> ________________________________ >>>>> From: Sophie Blee-Goldman <sop...@confluent.io> >>>>> Sent: Friday, February 8, 2019 9:56 AM >>>>> To: dev@kafka.apache.org >>>>> Subject: [DISCUSS] KIP-428: Add in-memory window store >>>>> >>>>> Streams currently only has support for a RocksDB window store, but >>> users >>>>> have been requesting an in-memory version. This KIP introduces a >> design >>>> for >>>>> an in-memory window store implementation. >>>>> >>>>> >>>>> >>>> >>> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-428%3A+Add+in-memory+window+store >>>>> >>>> >>>> >>>> -- >>>> *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 >>> >> > >
signature.asc
Description: OpenPGP digital signature