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
> > > > > 
> > > > 
> > 


Reply via email to