Strong +1 to broker status API. It will be super helpful for other
operational use-cases.
If not too much extra effort, having "has leaders" or "number of
leaders" in there, will be useful too - it is only safe to modify the
network layer (change LB config, for instance) when a broker is no
longer the leader for any partition.
On Wed, Nov 13, 2019 at 8:43 AM Jason Gustafson <ja...@confluent.io> wrote:
>
> Ni Noa,
>
> Thanks for the KIP. I agree with the concerns about Metadata bloat. In
> fact, I have wanted a BrokerStatus API for a while now. Perhaps this is a
> good excuse to introduce it. I was thinking something like this:
>
> BrokerStatusRequest => BrokerId
>
> BrokerStatusResponse =>
> Listeners => [ListenerName Host Port]
> RackId
> BrokerEpoch
> BrokerState => {ACTIVE, RECOVERING, SHUTTING_DOWN}
> UpTime
>
> What do you think?
>
> Thanks,
> Jason
>
> On Tue, Nov 5, 2019 at 12:25 PM Noa Resare <n...@resare.com> wrote:
>
> > I agree with that. And looking at the MetadataResponse fields it seems
> > there has been some feature creep already here. Does the client use rack
> > information, for example?
> >
> > I guess one could do this either by introducing a new leaner message pair,
> > used specifically enable client operation, and use
> > MetadataRequest/MetadataResponse for describeCluster() or one could shrink
> > MetadataRequest/MetadataResponse and introduce a new more fully featured
> > message pair for the other stuff. I would be happy to spend some time
> > looking into implementing this if there is an interest.
> >
> > /noa
> >
> > > On 5 Nov 2019, at 15:43, Gwen Shapira <g...@confluent.io> wrote:
> > >
> > > It isn't just about saving space. It increases complexity to default to
> > > always sharing a bit of information that is really only needed in a
> > single
> > > use-case.
> > > We avoid doing this as a matter of good protocol design.
> > > Arguably, this should not really piggyback on cluster metadata at all,
> > > since the usage is so different.
> > >
> > > On Tue, Nov 5, 2019, 7:29 AM Noa Resare <n...@resare.com> wrote:
> > >
> > >> It would certainly be possible to have the field be optional and only
> > >> include it if some flag is set in the DescribeClusterOptions instance
> > >> passed to Admin.describeCluster() that in turn would translate to a
> > boolean
> > >> in MetadataRequest indicating that we are asking for this piece of
> > >> information.
> > >>
> > >> I’m not entirely sure that this extra complexity would be worth it for
> > the
> > >> modestly smaller response size, in a message that already contains
> > things
> > >> like the hostname and rack identifier per broker.
> > >>
> > >> /noa
> > >>
> > >>> On 4 Nov 2019, at 14:45, Gwen Shapira <g...@confluent.io> wrote:
> > >>>
> > >>> Cluster metadata is sent to clients on a very regular basis. Adding
> > >>> start-time there seems quite repetitive. Especially considering that
> > >>> this information is only useful in very specific cases.
> > >>>
> > >>> Can we add this capability to the Admin API in a way that won't impact
> > >>> normal client workflow?
> > >>>
> > >>> On Mon, Nov 4, 2019 at 4:05 AM Noa Resare <n...@resare.com> wrote:
> > >>>>
> > >>>> Thank you for the feedback, Stanislav!
> > >>>>
> > >>>> I agree that it would be good to harmonise the naming, and
> > >> start-time-ms definitely more descriptive.
> > >>>>
> > >>>> I have updated the proposal to reflect this, and also added the
> > updated
> > >> json RPC changes. Please have a look.
> > >>>>
> > >>>> /noa
> > >>>>
> > >>>>> On 1 Nov 2019, at 09:13, Stanislav Kozlovski <stanis...@confluent.io
> > >
> > >> wrote:
> > >>>>>
> > >>>>> Hey Noa,
> > >>>>>
> > >>>>> KIP-436 added a JMX metric in Kafka for this exact use-case, called
> > >>>>> `start-time-ms`. Perhaps it would be useful to name this public
> > >> interface
> > >>>>> in the same way for consistency.
> > >>>>>
> > >>>>> Could you update the KIP to include the specific RPC changes
> > regarding
> > >> the
> > >>>>> metadata request/responses? Here is a recent example of how to
> > portray
> > >> the
> > >>>>> changes -
> > >>>>>
> > >>
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-525+-+Return+topic+metadata+and+configs+in+CreateTopics+response
> > >>>>>
> > >>>>> Thanks,
> > >>>>> Stanislav!
> > >>>>>
> > >>>>> On Mon, Oct 14, 2019 at 2:46 PM Noa Resare <n...@resare.com> wrote:
> > >>>>>
> > >>>>>> We are in the process of migrating the pieces of automation that
> > >> currently
> > >>>>>> reads and modifies zookeeper state to use the Admin API.
> > >>>>>>
> > >>>>>> One of the things that we miss doing this is access to the start
> > time
> > >> of
> > >>>>>> brokers in a cluster which is used by our automation doing rolling
> > >>>>>> restarts. We currently read this from the timestamp field from the
> > >>>>>> epehmeral broker znodes in zookeeper. To address this limitation, I
> > >> have
> > >>>>>> authored KIP-536, that proposes adding a timestamp field to the Node
> > >> class
> > >>>>>> that the AdminClient.describeCluster() method returns.
> > >>>>>>
> > >>>>>>
> > >>>>>>
> > >>
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-536%3A+Propagate+broker+timestamp+to+Admin+API
> > >>>>>> <
> > >>>>>>
> > >>
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-536:+Propagate+broker+timestamp+to+Admin+API
> > >>>>>>>
> > >>>>>>
> > >>>>>> Any and all feedback is most welcome
> > >>>>>>
> > >>>>>> cheers
> > >>>>>> noa
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>> --
> > >>>>> Best,
> > >>>>> Stanislav
> > >>>>
> > >>
> > >>
> >
> >
