Hi Noa,
I understand the desire for batching. However, once you do that, you either
need request forwarding or broker metadata propagation. It's worth
exploring, but I suspect you will end up with most of the changes needed by
the original proposal, in that case.
Ismael
On Mon, Nov 18, 2019 at 7:07 AM Noa Resare <n...@resare.com> wrote:
> Hi Jason & Gwen,
>
> Sure, this would solve our use case. I do have two questions, however:
>
> Moving from start time to uptime in theory would neatly side step the
> clock skew problem,
> but in practice I’m not sure it helps that much as the straightforward way
> of going about
> implementing this by returning (now - startTime) would break when the
> clock on the broker
> is adjusted. So, I think that startTime is more straight forward and
> doesn’t hide the fact that
> the system clock is sometimes off.
>
> Another thing I think would be useful is to build support for requesting
> multiple (or all?)
> brokers in a single request at the start. Having the request hold a set of
> brokerIds
> and return a set of brokers in the response, adding a brokerId field to
> identify which
> broker a specific broker status response is associated with seems straight
> forward.
>
> I’ll start writing a proposal so that we have something concrete to
> discuss.
>
> /noa
>
> > On 13 Nov 2019, at 16:43, 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
> >>>>>>
> >>>>
> >>>>
> >>
> >>
>
>
