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
> >>>>
> >>
> >>
>
>
