Sorry, that should be "Hi Noa"
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
>> >>>>
>> >>
>> >>
>>
>>
