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/08/27 21:19:02 UTC

[DISCUSS] KIP-771: KRaft broker should not expose controller metrics

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>.
Hi all,

I have made some edits to the KIP to expose controller metrics on standby
controllers. This may help the user troubleshoot misbehaving standby
controllers since the metrics of standby controllers could be lagging
behind the metrics of the active controller.

Here are the changes to the KIP:
https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=188743985&selectedPageVersions=16&selectedPageVersions=14

Regards,
Ryan Dielhenn

On Fri, Aug 27, 2021 at 2:19 PM Ryan Dielhenn <rd...@confluent.io>
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
> > > >
> > > 
> > 
> 

Re: [DISCUSS] KIP-771: KRaft broker should not expose controller metrics

Posted by Ryan Dielhenn <rd...@confluent.io.INVALID>.
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 <cm...@apache.org>.
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 Ron Dagostino <rn...@gmail.com>.
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
>