+1 again from me. On Thu, Jan 28, 2021 at 8:06 AM John Roesler <vvcep...@apache.org> wrote:
> Thank you for the KIP, Bruno! > > I'm +1 (binding) > > -John > > On Wed, 2021-01-27 at 14:18 -0600, Leah Thomas wrote: > > 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 > > > > > > > > > > > > > > > > > -- -- Guozhang
