Interesting... I did not initially consider it because I didn't want to
have an impact on anyone's Streams apps, but now I see that unless
developers have subclassed `Windows`, the number of segments would always
be 3!

There's one caveat to this, which I think was a mistake. The field
`segments` in Windows is public, which means that anyone can actually set
it directly on any Window instance like:

        TimeWindows tw = TimeWindows.of(100);
        tw.segments = 12345;

Bypassing the bounds check and contradicting the javadoc in Windows that
says users can't directly set it. Sadly there's no way to "deprecate" this
exposure, so I propose just to make it private.

With this new knowledge, I agree, I think we can switch to
"segmentInterval" throughout the interface.

On Wed, Jun 20, 2018 at 5:06 PM Guozhang Wang <wangg...@gmail.com> wrote:

> Hello John,
>
> Thanks for the KIP.
>
> Should we consider making the change on `Stores#persistentWindowStore`
> parameters as well?
>
>
> Guozhang
>
>
> On Wed, Jun 20, 2018 at 1:31 PM, John Roesler <j...@confluent.io> wrote:
>
> > Hi Ted,
> >
> > Ah, when you made that comment to me before, I thought you meant as
> opposed
> > to "segments". Now it makes sense that you meant as opposed to
> > "segmentSize".
> >
> > I named it that way to match the peer method "windowSize", which is also
> a
> > quantity of milliseconds.
> >
> > I agree that "interval" is more intuitive, but I think I favor
> consistency
> > in this case. Does that seem reasonable?
> >
> > Thanks,
> > -John
> >
> > On Wed, Jun 20, 2018 at 1:06 PM Ted Yu <yuzhih...@gmail.com> wrote:
> >
> > > Normally size is not measured in time unit, such as milliseconds.
> > > How about naming the new method segmentInterval ?
> > > Thanks
> > > -------- Original message --------From: John Roesler <
> j...@confluent.io>
> > > Date: 6/20/18  10:45 AM  (GMT-08:00) To: dev@kafka.apache.org Subject:
> > > [DISCUSS] KIP-319: Replace segments with segmentSize in
> > > WindowBytesStoreSupplier
> > > Hello All,
> > >
> > > I'd like to propose KIP-319 to fix an issue I identified in KAFKA-7080.
> > > Specifically, we're creating CachingWindowStore with the *number of
> > > segments* instead of the *segment size*.
> > >
> > > Here's the jira: https://issues.apache.org/jira/browse/KAFKA-7080
> > > Here's the KIP: https://cwiki.apache.org/confluence/x/mQU0BQ
> > >
> > > additionally, here's a draft PR for clarity:
> > > https://github.com/apache/kafka/pull/5257
> > >
> > > Please let me know what you think!
> > >
> > > Thanks,
> > > -John
> > >
> >
>
>
>
> --
> -- Guozhang
>

Reply via email to