You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Kevin Lu <lu...@berkeley.edu> on 2018/11/05 15:04:02 UTC
Re: [DISCUSS] KIP-351: Add --under-min-isr option to describe topics command
Bumping this to hopefully get a couple more opinions.
Since KIP-377 has been accepted (it adds the --bootstrap-server option
needed for this KIP), do we think it is okay to proceed to vote on this KIP
without KIP-377 fully merged yet?
Thanks.
Regards,
Kevin
On Mon, Oct 22, 2018 at 6:32 PM Kevin Lu <lu...@berkeley.edu> wrote:
> Hi Mickael,
>
> Thanks for the suggestion for getting the "computed" configuration from
> AdminClient.describeConfigs(). That's exactly what I was looking for!
>
> I have updated the KIP to use AdminClient.describeConfigs(), and included
> a code snippet. Please take a look.
>
> Since KIP-377 proposes the same bootstrap-server option, I've put a
> dependency for this KIP to have KIP-377 implemented first so we don't
> duplicate work or introduce conflicts. I'll give the other thread a bump,
> but I think we can still continue discussion/voting for this KIP.
>
> Thoughts?
>
> Regards,
> Kevin
>
> On Mon, Oct 22, 2018 at 4:01 AM Mickael Maison <mi...@gmail.com>
> wrote:
>
>> It might be worth syncing with KIP-377 which is already planning to
>> make TopicCommand use the AdminClient and add a --bootstrap-server
>> argument.
>>
>> Also in the proposed changes section, you mention the challenge of
>> finding the topic min ISR configuration. Using the
>> AdminClient.describeConfigs() API,
>> you directly get the "computed" configuration for topics. If the topic
>> is using the default config from the broker the configuration source
>> will be set to "DEFAULT_CONFIG". In case, the configuration was
>> specified during creation, the source will be set to
>> "DYNAMIC_TOPIC_CONFIG". So there's no need to query Zookeeper.
>> On Fri, Oct 19, 2018 at 5:02 PM Kevin Lu <lu...@berkeley.edu> wrote:
>> >
>> > Bumping this as I have added some additional details.
>> >
>> > This change will require adding a "--bootstrap-server" flag to identify
>> the
>> > current broker/cluster configured "min.insync.replicas".
>> >
>> > Regards,
>> > Kevin
>> >
>> > On Fri, Oct 12, 2018 at 4:19 PM Kevin Lu <lu...@berkeley.edu> wrote:
>> >
>> > > Hi All,
>> > >
>> > > After some feedback, I have reformulated KIP-351
>> > > <
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-351%3A+Add+--under-min-isr+option+to+describe+topics+command
>> >
>> > > .
>> > >
>> > > This KIP proposes an additional "--under-min-isr" option in
>> TopicCommand
>> > > to show topic partitions which are under the configured
>> > > "min.insync.replicas" to help operators identify which topic
>> partitions
>> > > need immediate fixing.
>> > >
>> > > Please take a look and provide some feedback!
>> > >
>> > > Thanks!
>> > >
>> > > Regards,
>> > > Kevin
>> > >
>>
>
Re: [DISCUSS] KIP-351: Add --under-min-isr option to describe topics command
Posted by Kevin Lu <lu...@berkeley.edu>.
Hi Stanislav,
Thanks for the response.
I will give it a few more days to let others look at the KIP before putting
it to a vote then.
Regards,
Kevin
On Mon, Nov 5, 2018 at 7:32 AM Stanislav Kozlovski <st...@confluent.io>
wrote:
> Hey Kevin,
>
> Thanks for the KIP! This sounds like a very useful feature to have and
> makes excellent use of the new AdminClient introduced in KIP-377 in my
> opinion.
> I believe it's perfectly fine to start a vote thread on the fact that
> KIP-377 was accepted already.
>
> On Mon, Nov 5, 2018 at 3:03 PM Kevin Lu <lu...@berkeley.edu> wrote:
>
> > Bumping this to hopefully get a couple more opinions.
> >
> > Since KIP-377 has been accepted (it adds the --bootstrap-server option
> > needed for this KIP), do we think it is okay to proceed to vote on this
> KIP
> > without KIP-377 fully merged yet?
> >
> > Thanks.
> >
> > Regards,
> > Kevin
> >
> > On Mon, Oct 22, 2018 at 6:32 PM Kevin Lu <lu...@berkeley.edu> wrote:
> >
> > > Hi Mickael,
> > >
> > > Thanks for the suggestion for getting the "computed" configuration from
> > > AdminClient.describeConfigs(). That's exactly what I was looking for!
> > >
> > > I have updated the KIP to use AdminClient.describeConfigs(), and
> included
> > > a code snippet. Please take a look.
> > >
> > > Since KIP-377 proposes the same bootstrap-server option, I've put a
> > > dependency for this KIP to have KIP-377 implemented first so we don't
> > > duplicate work or introduce conflicts. I'll give the other thread a
> bump,
> > > but I think we can still continue discussion/voting for this KIP.
> > >
> > > Thoughts?
> > >
> > > Regards,
> > > Kevin
> > >
> > > On Mon, Oct 22, 2018 at 4:01 AM Mickael Maison <
> mickael.maison@gmail.com
> > >
> > > wrote:
> > >
> > >> It might be worth syncing with KIP-377 which is already planning to
> > >> make TopicCommand use the AdminClient and add a --bootstrap-server
> > >> argument.
> > >>
> > >> Also in the proposed changes section, you mention the challenge of
> > >> finding the topic min ISR configuration. Using the
> > >> AdminClient.describeConfigs() API,
> > >> you directly get the "computed" configuration for topics. If the topic
> > >> is using the default config from the broker the configuration source
> > >> will be set to "DEFAULT_CONFIG". In case, the configuration was
> > >> specified during creation, the source will be set to
> > >> "DYNAMIC_TOPIC_CONFIG". So there's no need to query Zookeeper.
> > >> On Fri, Oct 19, 2018 at 5:02 PM Kevin Lu <lu...@berkeley.edu>
> wrote:
> > >> >
> > >> > Bumping this as I have added some additional details.
> > >> >
> > >> > This change will require adding a "--bootstrap-server" flag to
> > identify
> > >> the
> > >> > current broker/cluster configured "min.insync.replicas".
> > >> >
> > >> > Regards,
> > >> > Kevin
> > >> >
> > >> > On Fri, Oct 12, 2018 at 4:19 PM Kevin Lu <lu...@berkeley.edu>
> > wrote:
> > >> >
> > >> > > Hi All,
> > >> > >
> > >> > > After some feedback, I have reformulated KIP-351
> > >> > > <
> > >>
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-351%3A+Add+--under-min-isr+option+to+describe+topics+command
> > >> >
> > >> > > .
> > >> > >
> > >> > > This KIP proposes an additional "--under-min-isr" option in
> > >> TopicCommand
> > >> > > to show topic partitions which are under the configured
> > >> > > "min.insync.replicas" to help operators identify which topic
> > >> partitions
> > >> > > need immediate fixing.
> > >> > >
> > >> > > Please take a look and provide some feedback!
> > >> > >
> > >> > > Thanks!
> > >> > >
> > >> > > Regards,
> > >> > > Kevin
> > >> > >
> > >>
> > >
> >
>
>
> --
> Best,
> Stanislav
>
Re: [DISCUSS] KIP-351: Add --under-min-isr option to describe topics command
Posted by Stanislav Kozlovski <st...@confluent.io>.
Hey Kevin,
Thanks for the KIP! This sounds like a very useful feature to have and
makes excellent use of the new AdminClient introduced in KIP-377 in my
opinion.
I believe it's perfectly fine to start a vote thread on the fact that
KIP-377 was accepted already.
On Mon, Nov 5, 2018 at 3:03 PM Kevin Lu <lu...@berkeley.edu> wrote:
> Bumping this to hopefully get a couple more opinions.
>
> Since KIP-377 has been accepted (it adds the --bootstrap-server option
> needed for this KIP), do we think it is okay to proceed to vote on this KIP
> without KIP-377 fully merged yet?
>
> Thanks.
>
> Regards,
> Kevin
>
> On Mon, Oct 22, 2018 at 6:32 PM Kevin Lu <lu...@berkeley.edu> wrote:
>
> > Hi Mickael,
> >
> > Thanks for the suggestion for getting the "computed" configuration from
> > AdminClient.describeConfigs(). That's exactly what I was looking for!
> >
> > I have updated the KIP to use AdminClient.describeConfigs(), and included
> > a code snippet. Please take a look.
> >
> > Since KIP-377 proposes the same bootstrap-server option, I've put a
> > dependency for this KIP to have KIP-377 implemented first so we don't
> > duplicate work or introduce conflicts. I'll give the other thread a bump,
> > but I think we can still continue discussion/voting for this KIP.
> >
> > Thoughts?
> >
> > Regards,
> > Kevin
> >
> > On Mon, Oct 22, 2018 at 4:01 AM Mickael Maison <mickael.maison@gmail.com
> >
> > wrote:
> >
> >> It might be worth syncing with KIP-377 which is already planning to
> >> make TopicCommand use the AdminClient and add a --bootstrap-server
> >> argument.
> >>
> >> Also in the proposed changes section, you mention the challenge of
> >> finding the topic min ISR configuration. Using the
> >> AdminClient.describeConfigs() API,
> >> you directly get the "computed" configuration for topics. If the topic
> >> is using the default config from the broker the configuration source
> >> will be set to "DEFAULT_CONFIG". In case, the configuration was
> >> specified during creation, the source will be set to
> >> "DYNAMIC_TOPIC_CONFIG". So there's no need to query Zookeeper.
> >> On Fri, Oct 19, 2018 at 5:02 PM Kevin Lu <lu...@berkeley.edu> wrote:
> >> >
> >> > Bumping this as I have added some additional details.
> >> >
> >> > This change will require adding a "--bootstrap-server" flag to
> identify
> >> the
> >> > current broker/cluster configured "min.insync.replicas".
> >> >
> >> > Regards,
> >> > Kevin
> >> >
> >> > On Fri, Oct 12, 2018 at 4:19 PM Kevin Lu <lu...@berkeley.edu>
> wrote:
> >> >
> >> > > Hi All,
> >> > >
> >> > > After some feedback, I have reformulated KIP-351
> >> > > <
> >>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-351%3A+Add+--under-min-isr+option+to+describe+topics+command
> >> >
> >> > > .
> >> > >
> >> > > This KIP proposes an additional "--under-min-isr" option in
> >> TopicCommand
> >> > > to show topic partitions which are under the configured
> >> > > "min.insync.replicas" to help operators identify which topic
> >> partitions
> >> > > need immediate fixing.
> >> > >
> >> > > Please take a look and provide some feedback!
> >> > >
> >> > > Thanks!
> >> > >
> >> > > Regards,
> >> > > Kevin
> >> > >
> >>
> >
>
--
Best,
Stanislav