You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Randall Hauch <rh...@gmail.com> on 2020/04/30 21:59:32 UTC
[DISCUSS] KIP-605 Expand Connect Worker Internal Topic Settings
Hello!
I'd like to use this thread to discuss KIP-605, which expands some of the
properties that the Connect distributed worker uses when creating internal
topics:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-605%3A+Expand+Connect+Worker+Internal+Topic+Settings
Best regards,
Randall
Re: [DISCUSS] KIP-605 Expand Connect Worker Internal Topic Settings
Posted by Randall Hauch <rh...@gmail.com>.
Thanks, folks.
The KIP appears to be satisfactory, so I plan to start a vote thread
tomorrow unless there are objections.
Best regards,
Randall
On Mon, May 4, 2020 at 2:56 PM Christopher Egerton <ch...@confluent.io>
wrote:
> Hi Randall,
>
> Looks great! Definitely in favor of a WARN level message instead of failing
> on startup. +1 non-binding when the vote thread comes
>
> Cheers,
>
> Chris
>
> On Mon, May 4, 2020 at 12:51 PM Randall Hauch <rh...@gmail.com> wrote:
>
> > Thanks, Chris.
> >
> > 1. Added a use case that describes why you might want to use -1 for
> > replication factor but want to set other properties.
> > 2&3. Thanks for bringing up these special properties that the worker
> always
> > sets. I've added an "Excluded properties" column to the table of new
> > properties, and listed out the topic-specific properties should not be
> set.
> > I've also added this paragraph:
> >
> > Note that some topic-specific properties are excluded because the
> > distributed worker always sets specific values. Therefore, if a
> distributed
> > worker configuration does set any of these excluded properties, the
> > distributed worker will issue a warning that such properties should not
> be
> > set and will be ignored.
> >
> >
> > I'm proposing that using such properties will result in a WARN log
> message,
> > but allow the worker to continue by ignoring these values. I added a
> > rejected alternative describing why I think failing is not desirable.
> >
> > 4. Added a sentence as suggested.
> >
> > Thanks!
> >
> > On Sun, May 3, 2020 at 12:55 PM Christopher Egerton <chrise@confluent.io
> >
> > wrote:
> >
> > > Hi Randall,
> > >
> > > Thanks for the KIP! I have a few questions and suggestions but no major
> > > objections.
> > >
> > > 1. The motivation is pretty clear for altering the various
> > > "*.storage.replication.factor" properties to allow -1 as a value now.
> Are
> > > there expected use cases for allowing modification of other properties
> of
> > > these topic configs? It'd be nice to understand why we're adding this
> > extra
> > > configurability to the worker.
> > >
> > > 2. Should the "cleanup.policy" property have some additional guarding
> > logic
> > > to make sure that people don't set it to "delete" or "both"?
> > >
> > > 3. The lack of a "config.storage.partitions" property seems intentional
> > > because the config topic should only ever have one partition. Now that
> > > we're adding all of these other internal topic-related properties, do
> you
> > > think it might be helpful to users if we emit a warning message of some
> > > sort when they try to configure their worker with this property?
> > >
> > > 4. On the topic of compatibility--this is a fairly niche edge case, but
> > any
> > > time we add new configs to the worker we run the risk of overlap with
> > > existing configs for REST extensions that users may have implemented.
> > This
> > > is different from other pluggable interfaces like config providers and
> > > converters, whose properties are namespaced (presumably to avoid
> > collisions
> > > like this). Might be worth it to note this in a small paragraph or even
> > > just a single sentence.
> > >
> > > Cheers,
> > >
> > > Chris
> > >
> > > On Thu, Apr 30, 2020 at 4:32 PM Ryanne Dolan <ry...@gmail.com>
> > > wrote:
> > >
> > > > Much needed, thanks.
> > > >
> > > > Ryanne
> > > >
> > > > On Thu, Apr 30, 2020 at 4:59 PM Randall Hauch <rh...@gmail.com>
> > wrote:
> > > >
> > > > > Hello!
> > > > >
> > > > > I'd like to use this thread to discuss KIP-605, which expands some
> of
> > > the
> > > > > properties that the Connect distributed worker uses when creating
> > > > internal
> > > > > topics:
> > > > >
> > > > >
> > > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-605%3A+Expand+Connect+Worker+Internal+Topic+Settings
> > > > >
> > > > > Best regards,
> > > > >
> > > > > Randall
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-605 Expand Connect Worker Internal Topic Settings
Posted by Christopher Egerton <ch...@confluent.io>.
Hi Randall,
Looks great! Definitely in favor of a WARN level message instead of failing
on startup. +1 non-binding when the vote thread comes
Cheers,
Chris
On Mon, May 4, 2020 at 12:51 PM Randall Hauch <rh...@gmail.com> wrote:
> Thanks, Chris.
>
> 1. Added a use case that describes why you might want to use -1 for
> replication factor but want to set other properties.
> 2&3. Thanks for bringing up these special properties that the worker always
> sets. I've added an "Excluded properties" column to the table of new
> properties, and listed out the topic-specific properties should not be set.
> I've also added this paragraph:
>
> Note that some topic-specific properties are excluded because the
> distributed worker always sets specific values. Therefore, if a distributed
> worker configuration does set any of these excluded properties, the
> distributed worker will issue a warning that such properties should not be
> set and will be ignored.
>
>
> I'm proposing that using such properties will result in a WARN log message,
> but allow the worker to continue by ignoring these values. I added a
> rejected alternative describing why I think failing is not desirable.
>
> 4. Added a sentence as suggested.
>
> Thanks!
>
> On Sun, May 3, 2020 at 12:55 PM Christopher Egerton <ch...@confluent.io>
> wrote:
>
> > Hi Randall,
> >
> > Thanks for the KIP! I have a few questions and suggestions but no major
> > objections.
> >
> > 1. The motivation is pretty clear for altering the various
> > "*.storage.replication.factor" properties to allow -1 as a value now. Are
> > there expected use cases for allowing modification of other properties of
> > these topic configs? It'd be nice to understand why we're adding this
> extra
> > configurability to the worker.
> >
> > 2. Should the "cleanup.policy" property have some additional guarding
> logic
> > to make sure that people don't set it to "delete" or "both"?
> >
> > 3. The lack of a "config.storage.partitions" property seems intentional
> > because the config topic should only ever have one partition. Now that
> > we're adding all of these other internal topic-related properties, do you
> > think it might be helpful to users if we emit a warning message of some
> > sort when they try to configure their worker with this property?
> >
> > 4. On the topic of compatibility--this is a fairly niche edge case, but
> any
> > time we add new configs to the worker we run the risk of overlap with
> > existing configs for REST extensions that users may have implemented.
> This
> > is different from other pluggable interfaces like config providers and
> > converters, whose properties are namespaced (presumably to avoid
> collisions
> > like this). Might be worth it to note this in a small paragraph or even
> > just a single sentence.
> >
> > Cheers,
> >
> > Chris
> >
> > On Thu, Apr 30, 2020 at 4:32 PM Ryanne Dolan <ry...@gmail.com>
> > wrote:
> >
> > > Much needed, thanks.
> > >
> > > Ryanne
> > >
> > > On Thu, Apr 30, 2020 at 4:59 PM Randall Hauch <rh...@gmail.com>
> wrote:
> > >
> > > > Hello!
> > > >
> > > > I'd like to use this thread to discuss KIP-605, which expands some of
> > the
> > > > properties that the Connect distributed worker uses when creating
> > > internal
> > > > topics:
> > > >
> > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-605%3A+Expand+Connect+Worker+Internal+Topic+Settings
> > > >
> > > > Best regards,
> > > >
> > > > Randall
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-605 Expand Connect Worker Internal Topic Settings
Posted by Randall Hauch <rh...@gmail.com>.
Thanks, Chris.
1. Added a use case that describes why you might want to use -1 for
replication factor but want to set other properties.
2&3. Thanks for bringing up these special properties that the worker always
sets. I've added an "Excluded properties" column to the table of new
properties, and listed out the topic-specific properties should not be set.
I've also added this paragraph:
Note that some topic-specific properties are excluded because the
distributed worker always sets specific values. Therefore, if a distributed
worker configuration does set any of these excluded properties, the
distributed worker will issue a warning that such properties should not be
set and will be ignored.
I'm proposing that using such properties will result in a WARN log message,
but allow the worker to continue by ignoring these values. I added a
rejected alternative describing why I think failing is not desirable.
4. Added a sentence as suggested.
Thanks!
On Sun, May 3, 2020 at 12:55 PM Christopher Egerton <ch...@confluent.io>
wrote:
> Hi Randall,
>
> Thanks for the KIP! I have a few questions and suggestions but no major
> objections.
>
> 1. The motivation is pretty clear for altering the various
> "*.storage.replication.factor" properties to allow -1 as a value now. Are
> there expected use cases for allowing modification of other properties of
> these topic configs? It'd be nice to understand why we're adding this extra
> configurability to the worker.
>
> 2. Should the "cleanup.policy" property have some additional guarding logic
> to make sure that people don't set it to "delete" or "both"?
>
> 3. The lack of a "config.storage.partitions" property seems intentional
> because the config topic should only ever have one partition. Now that
> we're adding all of these other internal topic-related properties, do you
> think it might be helpful to users if we emit a warning message of some
> sort when they try to configure their worker with this property?
>
> 4. On the topic of compatibility--this is a fairly niche edge case, but any
> time we add new configs to the worker we run the risk of overlap with
> existing configs for REST extensions that users may have implemented. This
> is different from other pluggable interfaces like config providers and
> converters, whose properties are namespaced (presumably to avoid collisions
> like this). Might be worth it to note this in a small paragraph or even
> just a single sentence.
>
> Cheers,
>
> Chris
>
> On Thu, Apr 30, 2020 at 4:32 PM Ryanne Dolan <ry...@gmail.com>
> wrote:
>
> > Much needed, thanks.
> >
> > Ryanne
> >
> > On Thu, Apr 30, 2020 at 4:59 PM Randall Hauch <rh...@gmail.com> wrote:
> >
> > > Hello!
> > >
> > > I'd like to use this thread to discuss KIP-605, which expands some of
> the
> > > properties that the Connect distributed worker uses when creating
> > internal
> > > topics:
> > >
> > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-605%3A+Expand+Connect+Worker+Internal+Topic+Settings
> > >
> > > Best regards,
> > >
> > > Randall
> > >
> >
>
Re: [DISCUSS] KIP-605 Expand Connect Worker Internal Topic Settings
Posted by Konstantine Karantasis <ko...@confluent.io>.
Thanks for the KIP Randall!
Automatic creation of internal Connect topics has been very useful since
KIP-154 and adapting the Connect workers to allow users to transparently
use broker defaults will be useful as well.
The KIP looks good.
Konstantine
On Sun, May 3, 2020 at 10:55 AM Christopher Egerton <ch...@confluent.io>
wrote:
> Hi Randall,
>
> Thanks for the KIP! I have a few questions and suggestions but no major
> objections.
>
> 1. The motivation is pretty clear for altering the various
> "*.storage.replication.factor" properties to allow -1 as a value now. Are
> there expected use cases for allowing modification of other properties of
> these topic configs? It'd be nice to understand why we're adding this extra
> configurability to the worker.
>
> 2. Should the "cleanup.policy" property have some additional guarding logic
> to make sure that people don't set it to "delete" or "both"?
>
> 3. The lack of a "config.storage.partitions" property seems intentional
> because the config topic should only ever have one partition. Now that
> we're adding all of these other internal topic-related properties, do you
> think it might be helpful to users if we emit a warning message of some
> sort when they try to configure their worker with this property?
>
> 4. On the topic of compatibility--this is a fairly niche edge case, but any
> time we add new configs to the worker we run the risk of overlap with
> existing configs for REST extensions that users may have implemented. This
> is different from other pluggable interfaces like config providers and
> converters, whose properties are namespaced (presumably to avoid collisions
> like this). Might be worth it to note this in a small paragraph or even
> just a single sentence.
>
> Cheers,
>
> Chris
>
> On Thu, Apr 30, 2020 at 4:32 PM Ryanne Dolan <ry...@gmail.com>
> wrote:
>
> > Much needed, thanks.
> >
> > Ryanne
> >
> > On Thu, Apr 30, 2020 at 4:59 PM Randall Hauch <rh...@gmail.com> wrote:
> >
> > > Hello!
> > >
> > > I'd like to use this thread to discuss KIP-605, which expands some of
> the
> > > properties that the Connect distributed worker uses when creating
> > internal
> > > topics:
> > >
> > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-605%3A+Expand+Connect+Worker+Internal+Topic+Settings
> > >
> > > Best regards,
> > >
> > > Randall
> > >
> >
>
Re: [DISCUSS] KIP-605 Expand Connect Worker Internal Topic Settings
Posted by Christopher Egerton <ch...@confluent.io>.
Hi Randall,
Thanks for the KIP! I have a few questions and suggestions but no major
objections.
1. The motivation is pretty clear for altering the various
"*.storage.replication.factor" properties to allow -1 as a value now. Are
there expected use cases for allowing modification of other properties of
these topic configs? It'd be nice to understand why we're adding this extra
configurability to the worker.
2. Should the "cleanup.policy" property have some additional guarding logic
to make sure that people don't set it to "delete" or "both"?
3. The lack of a "config.storage.partitions" property seems intentional
because the config topic should only ever have one partition. Now that
we're adding all of these other internal topic-related properties, do you
think it might be helpful to users if we emit a warning message of some
sort when they try to configure their worker with this property?
4. On the topic of compatibility--this is a fairly niche edge case, but any
time we add new configs to the worker we run the risk of overlap with
existing configs for REST extensions that users may have implemented. This
is different from other pluggable interfaces like config providers and
converters, whose properties are namespaced (presumably to avoid collisions
like this). Might be worth it to note this in a small paragraph or even
just a single sentence.
Cheers,
Chris
On Thu, Apr 30, 2020 at 4:32 PM Ryanne Dolan <ry...@gmail.com> wrote:
> Much needed, thanks.
>
> Ryanne
>
> On Thu, Apr 30, 2020 at 4:59 PM Randall Hauch <rh...@gmail.com> wrote:
>
> > Hello!
> >
> > I'd like to use this thread to discuss KIP-605, which expands some of the
> > properties that the Connect distributed worker uses when creating
> internal
> > topics:
> >
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-605%3A+Expand+Connect+Worker+Internal+Topic+Settings
> >
> > Best regards,
> >
> > Randall
> >
>
Re: [DISCUSS] KIP-605 Expand Connect Worker Internal Topic Settings
Posted by Ryanne Dolan <ry...@gmail.com>.
Much needed, thanks.
Ryanne
On Thu, Apr 30, 2020 at 4:59 PM Randall Hauch <rh...@gmail.com> wrote:
> Hello!
>
> I'd like to use this thread to discuss KIP-605, which expands some of the
> properties that the Connect distributed worker uses when creating internal
> topics:
>
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-605%3A+Expand+Connect+Worker+Internal+Topic+Settings
>
> Best regards,
>
> Randall
>