You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by James Cheng <wu...@gmail.com> on 2020/04/01 04:29:13 UTC

Re: [DISCUSS] KIP-579: new exception on min.insync.replicas > replication.factor

I agree that we should prevent the creation of such a topic configuration. That would mean catching it at topic-creation time, as well as catching it on any topic-configuration changes that might make min.isr > replication factor.

Not sure how we would detect things if someone changed the broker-default configuration. That could be tricky.

Btw, I was the person who filed the original JIRA and as Mickael guessed, it was done by mistake.

-James

> On Mar 31, 2020, at 9:30 AM, Ismael Juma <is...@juma.me.uk> wrote:
> 
> Hi Paolo,
> 
> Thanks for the KIP. Why would one want to set min.isr to be higher than
> replication factor even in that case? Mickael's suggestion seems better to
> me.
> 
> Ismael
> 
> On Fri, Mar 13, 2020 at 10:28 AM Paolo Moriello <pa...@gmail.com>
> wrote:
> 
>> Hi Mickael,
>> 
>> Thanks for your interest in this. The main motivation to NOT make topic
>> creation fail when this mismatch happens is because at the moment it is
>> possible to produce/consume on topics if acks is not set to all. I'm not
>> sure we want to disable this behavior (as we would by failing at topic
>> creation). That's why I decided to go for a softer approach, which at least
>> gives some more clarity to the users and avoids other issues mentioned in
>> the KIP.
>> 
>> Let's see what others think!
>> 
>> On Fri, 13 Mar 2020 at 17:16, Mickael Maison <mi...@gmail.com>
>> wrote:
>> 
>>> Hi Paolo,
>>> 
>>> Thanks for looking at this issue. This can indeed be a source of
>> confusion.
>>> 
>>> I'm wondering if we should prevent the creation of topics with
>>> min.insync.replicas > replication.factor?
>>> You listed that as a rejected alternative because it requires more
>>> changes. However, I can't think of any scenarios where a user would
>>> want to create such a topic. I'm guessing it's probably always by
>>> mistake.
>>> 
>>> Let's see what other people think but I think it's worth checking what
>>> needs to be done if we wanted to prevent topics with bogus configs
>>> 
>>> On Fri, Mar 13, 2020 at 3:28 PM Paolo Moriello
>>> <pa...@gmail.com> wrote:
>>>> 
>>>> Hi,
>>>> 
>>>> Following this Jira ticket (
>>> https://issues.apache.org/jira/browse/KAFKA-4680),
>>>> I've created a proposal (
>>>> 
>>> 
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-579%3A+new+exception+on+min.insync.replicas+%3E+replication.factor
>>> )
>>>> to add a new exception/error to be used on min.insync.replicas >
>>>> replication.factor.
>>>> 
>>>> The proposal aims to introduce a new exception specific for the
>>>> configuration mismatch above to be used when producers requires acks =
>>> all.
>>>> At the moment we are using NotEnoughReplicaException, which is a
>>> retriable
>>>> exception and is used to fail on insync replicas < min isr. Plan is to
>>> have
>>>> a new, non-retriable exception, to separate the two cases.
>>>> 
>>>> I've also submitted a PR for the change mentioned above:
>>>> https://github.com/apache/kafka/pull/8225
>>>> 
>>>> Please have a look and let me know what you think.
>>>> 
>>>> Thanks,
>>>> Paolo
>>> 
>> 


Re: [DISCUSS] KIP-579: new exception on min.insync.replicas > replication.factor

Posted by James Cheng <wu...@gmail.com>.

> On Apr 2, 2020, at 4:27 AM, Paolo Moriello <pa...@gmail.com> wrote:
> 
> Hi,
> 
> Thanks a lot for your feedback, I really appreciate your help on this.
> 
> Given what you suggested, I will take some time to update the kip with a
> proposal to make invalid configuration requests FAIL. This involves
> checking multiple code paths, as James was saying, to eg.: validate topic
> creation, topic-configuration changes, partition reassignment and broker
> configuration setup.
> 
> Regarding the latter, do you have any suggestion on what's the best thing
> to do? For instance, we know that we can specify min.insync.replicas at
> cluster level. At the same time, we can also specify
> default.replication.factor. If there is an inconsistency with this setup,
> do we want to make kafka fail at startup or allow the users to overwrite it
> at a later point? (I believe we should be strict here and fail at startup).
> A similar question applies to offsets.topic.replication.factor.
> 

Paolo,

I haven't had a chance to think of it deeply, but your idea of having kafka fail at startup makes sense to me. I'd like one of the committers to chime in about that idea, too.

Some broker default settings can also be set dynamically during runtime, so you will also have to catch/reject those.

-James

> Thanks,
> Paolo
> 
> On Wed, 1 Apr 2020 at 05:29, James Cheng <wu...@gmail.com> wrote:
> 
>> I agree that we should prevent the creation of such a topic configuration.
>> That would mean catching it at topic-creation time, as well as catching it
>> on any topic-configuration changes that might make min.isr > replication
>> factor.
>> 
>> Not sure how we would detect things if someone changed the broker-default
>> configuration. That could be tricky.
>> 
>> Btw, I was the person who filed the original JIRA and as Mickael guessed,
>> it was done by mistake.
>> 
>> -James
>> 
>>> On Mar 31, 2020, at 9:30 AM, Ismael Juma <is...@juma.me.uk> wrote:
>>> 
>>> Hi Paolo,
>>> 
>>> Thanks for the KIP. Why would one want to set min.isr to be higher than
>>> replication factor even in that case? Mickael's suggestion seems better
>> to
>>> me.
>>> 
>>> Ismael
>>> 
>>> On Fri, Mar 13, 2020 at 10:28 AM Paolo Moriello <
>> paolomoriello94@gmail.com>
>>> wrote:
>>> 
>>>> Hi Mickael,
>>>> 
>>>> Thanks for your interest in this. The main motivation to NOT make topic
>>>> creation fail when this mismatch happens is because at the moment it is
>>>> possible to produce/consume on topics if acks is not set to all. I'm not
>>>> sure we want to disable this behavior (as we would by failing at topic
>>>> creation). That's why I decided to go for a softer approach, which at
>> least
>>>> gives some more clarity to the users and avoids other issues mentioned
>> in
>>>> the KIP.
>>>> 
>>>> Let's see what others think!
>>>> 
>>>> On Fri, 13 Mar 2020 at 17:16, Mickael Maison <mi...@gmail.com>
>>>> wrote:
>>>> 
>>>>> Hi Paolo,
>>>>> 
>>>>> Thanks for looking at this issue. This can indeed be a source of
>>>> confusion.
>>>>> 
>>>>> I'm wondering if we should prevent the creation of topics with
>>>>> min.insync.replicas > replication.factor?
>>>>> You listed that as a rejected alternative because it requires more
>>>>> changes. However, I can't think of any scenarios where a user would
>>>>> want to create such a topic. I'm guessing it's probably always by
>>>>> mistake.
>>>>> 
>>>>> Let's see what other people think but I think it's worth checking what
>>>>> needs to be done if we wanted to prevent topics with bogus configs
>>>>> 
>>>>> On Fri, Mar 13, 2020 at 3:28 PM Paolo Moriello
>>>>> <pa...@gmail.com> wrote:
>>>>>> 
>>>>>> Hi,
>>>>>> 
>>>>>> Following this Jira ticket (
>>>>> https://issues.apache.org/jira/browse/KAFKA-4680),
>>>>>> I've created a proposal (
>>>>>> 
>>>>> 
>>>> 
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-579%3A+new+exception+on+min.insync.replicas+%3E+replication.factor
>>>>> )
>>>>>> to add a new exception/error to be used on min.insync.replicas >
>>>>>> replication.factor.
>>>>>> 
>>>>>> The proposal aims to introduce a new exception specific for the
>>>>>> configuration mismatch above to be used when producers requires acks =
>>>>> all.
>>>>>> At the moment we are using NotEnoughReplicaException, which is a
>>>>> retriable
>>>>>> exception and is used to fail on insync replicas < min isr. Plan is to
>>>>> have
>>>>>> a new, non-retriable exception, to separate the two cases.
>>>>>> 
>>>>>> I've also submitted a PR for the change mentioned above:
>>>>>> https://github.com/apache/kafka/pull/8225
>>>>>> 
>>>>>> Please have a look and let me know what you think.
>>>>>> 
>>>>>> Thanks,
>>>>>> Paolo
>>>>> 
>>>> 
>> 
>> 


Re: [DISCUSS] KIP-579: new exception on min.insync.replicas > replication.factor

Posted by Paolo Moriello <pa...@gmail.com>.
Hi,

Thanks a lot for your feedback, I really appreciate your help on this.

Given what you suggested, I will take some time to update the kip with a
proposal to make invalid configuration requests FAIL. This involves
checking multiple code paths, as James was saying, to eg.: validate topic
creation, topic-configuration changes, partition reassignment and broker
configuration setup.

Regarding the latter, do you have any suggestion on what's the best thing
to do? For instance, we know that we can specify min.insync.replicas at
cluster level. At the same time, we can also specify
default.replication.factor. If there is an inconsistency with this setup,
do we want to make kafka fail at startup or allow the users to overwrite it
at a later point? (I believe we should be strict here and fail at startup).
A similar question applies to offsets.topic.replication.factor.

Thanks,
Paolo

On Wed, 1 Apr 2020 at 05:29, James Cheng <wu...@gmail.com> wrote:

> I agree that we should prevent the creation of such a topic configuration.
> That would mean catching it at topic-creation time, as well as catching it
> on any topic-configuration changes that might make min.isr > replication
> factor.
>
> Not sure how we would detect things if someone changed the broker-default
> configuration. That could be tricky.
>
> Btw, I was the person who filed the original JIRA and as Mickael guessed,
> it was done by mistake.
>
> -James
>
> > On Mar 31, 2020, at 9:30 AM, Ismael Juma <is...@juma.me.uk> wrote:
> >
> > Hi Paolo,
> >
> > Thanks for the KIP. Why would one want to set min.isr to be higher than
> > replication factor even in that case? Mickael's suggestion seems better
> to
> > me.
> >
> > Ismael
> >
> > On Fri, Mar 13, 2020 at 10:28 AM Paolo Moriello <
> paolomoriello94@gmail.com>
> > wrote:
> >
> >> Hi Mickael,
> >>
> >> Thanks for your interest in this. The main motivation to NOT make topic
> >> creation fail when this mismatch happens is because at the moment it is
> >> possible to produce/consume on topics if acks is not set to all. I'm not
> >> sure we want to disable this behavior (as we would by failing at topic
> >> creation). That's why I decided to go for a softer approach, which at
> least
> >> gives some more clarity to the users and avoids other issues mentioned
> in
> >> the KIP.
> >>
> >> Let's see what others think!
> >>
> >> On Fri, 13 Mar 2020 at 17:16, Mickael Maison <mi...@gmail.com>
> >> wrote:
> >>
> >>> Hi Paolo,
> >>>
> >>> Thanks for looking at this issue. This can indeed be a source of
> >> confusion.
> >>>
> >>> I'm wondering if we should prevent the creation of topics with
> >>> min.insync.replicas > replication.factor?
> >>> You listed that as a rejected alternative because it requires more
> >>> changes. However, I can't think of any scenarios where a user would
> >>> want to create such a topic. I'm guessing it's probably always by
> >>> mistake.
> >>>
> >>> Let's see what other people think but I think it's worth checking what
> >>> needs to be done if we wanted to prevent topics with bogus configs
> >>>
> >>> On Fri, Mar 13, 2020 at 3:28 PM Paolo Moriello
> >>> <pa...@gmail.com> wrote:
> >>>>
> >>>> Hi,
> >>>>
> >>>> Following this Jira ticket (
> >>> https://issues.apache.org/jira/browse/KAFKA-4680),
> >>>> I've created a proposal (
> >>>>
> >>>
> >>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-579%3A+new+exception+on+min.insync.replicas+%3E+replication.factor
> >>> )
> >>>> to add a new exception/error to be used on min.insync.replicas >
> >>>> replication.factor.
> >>>>
> >>>> The proposal aims to introduce a new exception specific for the
> >>>> configuration mismatch above to be used when producers requires acks =
> >>> all.
> >>>> At the moment we are using NotEnoughReplicaException, which is a
> >>> retriable
> >>>> exception and is used to fail on insync replicas < min isr. Plan is to
> >>> have
> >>>> a new, non-retriable exception, to separate the two cases.
> >>>>
> >>>> I've also submitted a PR for the change mentioned above:
> >>>> https://github.com/apache/kafka/pull/8225
> >>>>
> >>>> Please have a look and let me know what you think.
> >>>>
> >>>> Thanks,
> >>>> Paolo
> >>>
> >>
>
>