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" <[email protected]> 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" <[email protected]> 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
> > > > <[email protected]> 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
> > > > >
> > > > 
> > > 
> > 
> 

Reply via email to