You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Ryan Dielhenn <rd...@confluent.io.INVALID> on 2021/09/01 19:15:34 UTC
Re: [DISCUSS] KIP-771: KRaft broker should not expose controller metrics
Thank you Ron & Colin for the comments.
I have updated the KIP with the suggested changes: https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=188743985&selectedPageVersions=6&selectedPageVersions=5
Regards,
Ryan Dielhenn
On 2021/08/31 22:41:21, "Colin McCabe" <cm...@apache.org> wrote:
> Hi Ryan,
>
> Thanks for the KIP.
>
> Hmm, we don't really use the term "zookeeper brokers." That is confusing since ZK and Kafka are separate services. I would suggest a term like pre-KRaft brokers.
>
> > Zookeeper brokers currently register 0 for every controller metric.
>
> It's not 0 for every broker, is it? We should outline the circumstances when it's not 0 (i.e. one of these brokers is the active controller).
>
> > KRaft does not have this issue because processes with the "broker" role are never
> > elected as the active controller.
>
> This is somewhat misleading since a node could have bother controller and broker roles. Maybe a clearer way of writing this would be "nodes that are not eligible to become controllers."
>
> > Proposed Changes
> > Zookeeper brokers expose 0 for controller metrics. KRaft brokers should not.
>
> It seems like we should document what metrics standby controllers expose when in KRaft mode. It seems like the two options are exposing 0 for these metrics, or exposing a similar value to the active controller.
>
> best,
> Colin
>
>
> On Fri, Aug 27, 2021, at 14:30, Ron Dagostino wrote:
> > Thanks for the KIP, Ryan. I agree this makes sense. It also reflects the
> > state of affairs right now: KRaft nodes that do not have the controller
> > role currently do not expose these metrics. Assuming this KIP ends up
> > being accepted, we would then close KAFKA-13140 and its associated PR
> > https://github.com/apache/kafka/pull/11133.
> >
> > Ron
> >
> > On Fri, Aug 27, 2021 at 5:19 PM Ryan Dielhenn
> > <rd...@confluent.io.invalid> wrote:
> >
> > > Hello kafka devs,
> > >
> > > I would like to start a discussion on a KIP I have created to change how
> > > controller metrics are exposed for KRaft brokers.
> > >
> > > Here is the KIP:
> > >
> > >
> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP+771%3A+KRaft+brokers+should+not+expose+controller+metrics
> > >
> > > Regards,
> > > Ryan Dielhenn
> > >
> >
>
Re: [DISCUSS] KIP-771: KRaft broker should not expose controller metrics
Posted by Ryan Dielhenn <rd...@confluent.io.INVALID>.
Thanks for the suggestions Colin,
I updated the KIP, here are the changes: https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=188743985&selectedPageVersions=14&selectedPageVersions=13
Regards,
Ryan Dielhenn
On 2021/09/07 16:46:13, "Colin McCabe" <co...@cmccabe.xyz> wrote:
> Thanks for the update. You still have a lot of places that are written misleadingly. For example,
>
> > Pre-Kraft brokers currently register 0 for every controller metric.
>
> Someone reading this would wonder, if they're always 0, then why do we have these metrics? Please phrase this better to indicate that the metrics are not 0 when the node is the active controller.
>
> best,
> Colin
>
>
> On Wed, Sep 1, 2021, at 12:15, Ryan Dielhenn wrote:
> > Thank you Ron & Colin for the comments.
> >
> > I have updated the KIP with the suggested changes:
> > https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=188743985&selectedPageVersions=6&selectedPageVersions=5
> >
> > Regards,
> > Ryan Dielhenn
> >
> > On 2021/08/31 22:41:21, "Colin McCabe" <cm...@apache.org> wrote:
> > > Hi Ryan,
> > >
> > > Thanks for the KIP.
> > >
> > > Hmm, we don't really use the term "zookeeper brokers." That is confusing since ZK and Kafka are separate services. I would suggest a term like pre-KRaft brokers.
> > >
> > > > Zookeeper brokers currently register 0 for every controller metric.
> > >
> > > It's not 0 for every broker, is it? We should outline the circumstances when it's not 0 (i.e. one of these brokers is the active controller).
> > >
> > > > KRaft does not have this issue because processes with the "broker" role are never
> > > > elected as the active controller.
> > >
> > > This is somewhat misleading since a node could have bother controller and broker roles. Maybe a clearer way of writing this would be "nodes that are not eligible to become controllers."
> > >
> > > > Proposed Changes
> > > > Zookeeper brokers expose 0 for controller metrics. KRaft brokers should not.
> > >
> > > It seems like we should document what metrics standby controllers expose when in KRaft mode. It seems like the two options are exposing 0 for these metrics, or exposing a similar value to the active controller.
> > >
> > > best,
> > > Colin
> > >
> > >
> > > On Fri, Aug 27, 2021, at 14:30, Ron Dagostino wrote:
> > > > Thanks for the KIP, Ryan. I agree this makes sense. It also reflects the
> > > > state of affairs right now: KRaft nodes that do not have the controller
> > > > role currently do not expose these metrics. Assuming this KIP ends up
> > > > being accepted, we would then close KAFKA-13140 and its associated PR
> > > > https://github.com/apache/kafka/pull/11133.
> > > >
> > > > Ron
> > > >
> > > > On Fri, Aug 27, 2021 at 5:19 PM Ryan Dielhenn
> > > > <rd...@confluent.io.invalid> wrote:
> > > >
> > > > > Hello kafka devs,
> > > > >
> > > > > I would like to start a discussion on a KIP I have created to change how
> > > > > controller metrics are exposed for KRaft brokers.
> > > > >
> > > > > Here is the KIP:
> > > > >
> > > > >
> > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP+771%3A+KRaft+brokers+should+not+expose+controller+metrics
> > > > >
> > > > > Regards,
> > > > > Ryan Dielhenn
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-771: KRaft broker should not expose controller metrics
Posted by Colin McCabe <co...@cmccabe.xyz>.
Thanks for the update. You still have a lot of places that are written misleadingly. For example,
> Pre-Kraft brokers currently register 0 for every controller metric.
Someone reading this would wonder, if they're always 0, then why do we have these metrics? Please phrase this better to indicate that the metrics are not 0 when the node is the active controller.
best,
Colin
On Wed, Sep 1, 2021, at 12:15, Ryan Dielhenn wrote:
> Thank you Ron & Colin for the comments.
>
> I have updated the KIP with the suggested changes:
> https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=188743985&selectedPageVersions=6&selectedPageVersions=5
>
> Regards,
> Ryan Dielhenn
>
> On 2021/08/31 22:41:21, "Colin McCabe" <cm...@apache.org> wrote:
> > Hi Ryan,
> >
> > Thanks for the KIP.
> >
> > Hmm, we don't really use the term "zookeeper brokers." That is confusing since ZK and Kafka are separate services. I would suggest a term like pre-KRaft brokers.
> >
> > > Zookeeper brokers currently register 0 for every controller metric.
> >
> > It's not 0 for every broker, is it? We should outline the circumstances when it's not 0 (i.e. one of these brokers is the active controller).
> >
> > > KRaft does not have this issue because processes with the "broker" role are never
> > > elected as the active controller.
> >
> > This is somewhat misleading since a node could have bother controller and broker roles. Maybe a clearer way of writing this would be "nodes that are not eligible to become controllers."
> >
> > > Proposed Changes
> > > Zookeeper brokers expose 0 for controller metrics. KRaft brokers should not.
> >
> > It seems like we should document what metrics standby controllers expose when in KRaft mode. It seems like the two options are exposing 0 for these metrics, or exposing a similar value to the active controller.
> >
> > best,
> > Colin
> >
> >
> > On Fri, Aug 27, 2021, at 14:30, Ron Dagostino wrote:
> > > Thanks for the KIP, Ryan. I agree this makes sense. It also reflects the
> > > state of affairs right now: KRaft nodes that do not have the controller
> > > role currently do not expose these metrics. Assuming this KIP ends up
> > > being accepted, we would then close KAFKA-13140 and its associated PR
> > > https://github.com/apache/kafka/pull/11133.
> > >
> > > Ron
> > >
> > > On Fri, Aug 27, 2021 at 5:19 PM Ryan Dielhenn
> > > <rd...@confluent.io.invalid> wrote:
> > >
> > > > Hello kafka devs,
> > > >
> > > > I would like to start a discussion on a KIP I have created to change how
> > > > controller metrics are exposed for KRaft brokers.
> > > >
> > > > Here is the KIP:
> > > >
> > > >
> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP+771%3A+KRaft+brokers+should+not+expose+controller+metrics
> > > >
> > > > Regards,
> > > > Ryan Dielhenn
> > > >
> > >
> >
>