Hi Bruno, I'm still +1, non-binding. Thanks for the updates! Leah
On Wed, Jan 27, 2021 at 1:56 PM Matthias J. Sax <mj...@apache.org> wrote: > Thanks for updating the KIP. > > +1 (binding) > > > -Matthias > > On 1/27/21 10:19 AM, Bruno Cadonna wrote: > > Hi all, > > > > Thanks for voting! > > > > I updated the KIP with some additional feedback I got. > > > > If I do not hear anything from folks that have already voted in the next > > couple of days, I will assume their vote is still valid. You can also > > confirm your vote if you want. > > > > KIP: https://cwiki.apache.org/confluence/x/7CnZCQ > > > > Best, > > Bruno > > > > On 26.01.21 02:19, Sophie Blee-Goldman wrote: > >> Thanks for the KIP Bruno, +1 (binding) > >> > >> Sophie > >> > >> On Mon, Jan 25, 2021 at 11:23 AM Guozhang Wang <wangg...@gmail.com> > >> wrote: > >> > >>> Hey Bruno, > >>> > >>> Thanks for your response! > >>> > >>> 1) Yup I'm good with option a) as well. > >>> 2) Thanks! > >>> 3) Sounds good to me. I think it would not change any StreamThread > >>> implementation regarding capturing exceptions from consumer.poll() > >>> since it > >>> captures StreamsException as fatal. > >>> > >>> > >>> Guozhang > >>> > >>> On Wed, Dec 16, 2020 at 4:43 AM Bruno Cadonna <br...@confluent.io> > >>> wrote: > >>> > >>>> Hi Guozhang, > >>>> > >>>> Thank for the feedback! > >>>> > >>>> Please find my answers inline. > >>>> > >>>> Best, > >>>> Bruno > >>>> > >>>> > >>>> On 14.12.20 23:33, Guozhang Wang wrote: > >>>>> Hello Bruno, > >>>>> > >>>>> Just a few more questions about the KIP: > >>>>> > >>>>> 1) If the internal topics exist but the calculated num.partitions do > >>> not > >>>>> match the existing topics, what would Streams do; > >>>> > >>>> Good point! I missed to explicitly consider misconfigurations in the > >>>> KIP. > >>>> > >>>> I propose to throw a fatal error in this case during manual and > >>>> automatic initialization. For the fatal error, we have two options: > >>>> a) introduce a second exception besides MissingInternalTopicException, > >>>> e.g. MisconfiguredInternalTopicException > >>>> b) rename MissingInternalTopicException to > >>>> MissingOrMisconfiguredInternalTopicException and throw that in both > >>> cases. > >>>> > >>>> Since the process to react on such an exception user-side should be > >>>> similar, I am fine with option b). However, IMO option a) is a bit > >>>> cleaner. WDYT? > >>>> > >>>>> 2) Since `init()` is a blocking call (we only return after all topics > >>> are > >>>>> confirmed to be created), should we have a timeout for this call as > >>> well > >>>> or > >>>>> not; > >>>> > >>>> I will add an overload with a timeout to the KIP. > >>>> > >>>>> 3) If the configure is set to `MANUAL_SETUP`, then during rebalance > do > >>> we > >>>>> still check if number of partitions of the existing topic match or > >>>>> not; > >>>> if > >>>>> not, do we throw the newly added exception or throw a fatal > >>>>> StreamsException? Today we would throw the StreamsException from > >>> assign() > >>>>> which would be then thrown from consumer.poll() as a fatal error. > >>>>> > >>>> > >>>> Yes, I think we should check if the number of partitions match. I > >>>> propose to throw the newly added exception in the same way as we throw > >>>> now the MissingSourceTopicException, i.e., throw it from > >>>> consumer.poll(). WDYT? > >>>> > >>>>> Guozhang > >>>>> > >>>>> > >>>>> On Mon, Dec 14, 2020 at 12:47 PM John Roesler <vvcep...@apache.org> > >>>> wrote: > >>>>> > >>>>>> Thanks, Bruno! > >>>>>> > >>>>>> I'm +1 (binding) > >>>>>> > >>>>>> -John > >>>>>> > >>>>>> On Mon, 2020-12-14 at 09:57 -0600, Leah Thomas wrote: > >>>>>>> Thanks for the KIP Bruno, LGTM. +1 (non-binding) > >>>>>>> > >>>>>>> Cheers, > >>>>>>> Leah > >>>>>>> > >>>>>>> On Mon, Dec 14, 2020 at 4:29 AM Bruno Cadonna <br...@confluent.io> > >>>>>> wrote: > >>>>>>> > >>>>>>>> Hi, > >>>>>>>> > >>>>>>>> I'd like to start the voting on KIP-698 that proposes an explicit > >>> user > >>>>>>>> initialization of broker-side state for Kafka Streams instead of > >>>>>> letting > >>>>>>>> Kafka Streams setting up the broker-side state automatically > during > >>>>>>>> rebalance. Such an explicit initialization avoids possible data > >>>>>>>> loss > >>>>>>>> issues due to automatic initialization. > >>>>>>>> > >>>>>>>> https://cwiki.apache.org/confluence/x/7CnZCQ > >>>>>>>> > >>>>>>>> Best, > >>>>>>>> Bruno > >>>>>>>> > >>>>>> > >>>>>> > >>>>>> > >>>>> > >>>> > >>> > >>> > >>> -- > >>> -- Guozhang > >>> > >> >