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

Reply via email to