Hello Sönke, Thanks for the quick catch! I've fixed the test with this change.
Guozhang On Wed, Apr 3, 2019 at 5:53 AM Sönke Liebau <soenke.lie...@opencore.com.invalid> wrote: > Hi Guozhang, > > I've left a comment on the merged pull request, but not sure if you'll get > notified about that since the PR was already merged, so I'll write here as > well. > Setting this to -1 needs to be reflected in the test > shouldAddInternalTopicConfigForRepartitionTopics > as well, as this currently checks for Long.MAX_VALUE [1] and consistently > fails. > > Best regards, > Sönke > > [1] > > https://github.com/apache/kafka/blob/213466b3d4fd21b332c0b6882fea36cf1affef1c/streams/src/test/java/org/apache/kafka/streams/processor/internals/InternalTopologyBuilderTest.java#L647 > > > On Wed, Apr 3, 2019 at 2:07 AM Guozhang Wang <wangg...@gmail.com> wrote: > > > Hello folks, > > > > I'm closing this voting thread now, thanks to all who have provided your > > feedbacks! > > > > Here's a quick tally: > > > > Binding +1: 4 (Damian, Bill, Manikumar, Guozhang) > > Non-binding +1: (John, Mickael). > > > > > > Guozhang > > > > On Fri, Mar 29, 2019 at 11:32 AM Guozhang Wang <wangg...@gmail.com> > wrote: > > > > > Ah I see, my bad :) Yes that was the documented value in `TopicConfig`, > > > and I agree we should just change that as well. > > > > > > Will update the KIP. > > > > > > > > > > > > On Fri, Mar 29, 2019 at 11:27 AM Mickael Maison < > > mickael.mai...@gmail.com> > > > wrote: > > > > > >> Hi Guozhang, > > >> > > >> I know the KIP is about segments configuration but I'm talking about > > >> retention.ms which is also explicitly set on repartition topics > > >> > > >> > > > https://github.com/apache/kafka/blob/trunk/streams/src/main/java/org/apache/kafka/streams/processor/internals/RepartitionTopicConfig.java#L39 > > >> Streams is setting it to Long.MAX_VALUE, but -1 is the "documented" > > >> way to disable the time limit. That's why I said "for consistency" as > > >> in practice it's not going to change anything. > > >> > > >> On Fri, Mar 29, 2019 at 5:09 PM Guozhang Wang <wangg...@gmail.com> > > wrote: > > >> > > > >> > Hello Mickael, > > >> > > > >> > segment.ms default value in TopicConfig is 7 days, I think this is > a > > >> > sufficient default value. Do you have any motivations to set it to > -1? > > >> > > > >> > > > >> > Guozhang > > >> > > > >> > On Fri, Mar 29, 2019 at 9:42 AM Mickael Maison < > > >> mickael.mai...@gmail.com> > > >> > wrote: > > >> > > > >> > > +1 (non binding) > > >> > > For consistency, should we also set retention.ms to -1 instead of > > >> > > Long.MAX_VALUE? > > >> > > > > >> > > On Fri, Mar 29, 2019 at 3:59 PM Manikumar < > > manikumar.re...@gmail.com> > > >> > > wrote: > > >> > > > > > >> > > > +1 (binding) > > >> > > > > > >> > > > Thanks for the KIP. > > >> > > > > > >> > > > On Fri, Mar 29, 2019 at 9:04 PM Damian Guy < > damian....@gmail.com> > > >> wrote: > > >> > > > > > >> > > > > +1 > > >> > > > > > > >> > > > > On Fri, 29 Mar 2019 at 01:59, John Roesler <j...@confluent.io > > > > >> wrote: > > >> > > > > > > >> > > > > > +1 (nonbinding) from me. > > >> > > > > > > > >> > > > > > On Thu, Mar 28, 2019 at 7:08 PM Guozhang Wang < > > >> wangg...@gmail.com> > > >> > > > > wrote: > > >> > > > > > > > >> > > > > > > Hello folks, > > >> > > > > > > > > >> > > > > > > I'd like to directly start a voting thread on this simple > > KIP > > >> to > > >> > > change > > >> > > > > > the > > >> > > > > > > default override values for repartition topics: > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > >> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-443%3A+Return+to+default+segment.ms+and+segment.index.bytes+in+Streams+repartition+topics > > >> > > > > > > > > >> > > > > > > The related PR can be found here as well: > > >> > > > > > > https://github.com/apache/kafka/pull/6511 > > >> > > > > > > > > >> > > > > > > If you have any thoughts or feedbacks, they are more than > > >> welcomed > > >> > > as > > >> > > > > > well. > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > -- Guozhang > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > >> > > > >> > > > >> > -- > > >> > -- Guozhang > > >> > > > > > > > > > -- > > > -- Guozhang > > > > > > > > > -- > > -- Guozhang > > > > > -- > Sönke Liebau > Partner > Tel. +49 179 7940878 > OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880 Wedel - Germany > -- -- Guozhang
