You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Rajini Sivaram <ra...@gmail.com> on 2017/08/24 17:50:50 UTC
[VOTE] KIP-188 - Add new metrics to support health checks
Hi all,
I would like to start the vote on KIP-188 that adds additional metrics to
support health checks for Kafka Ops. Details are here:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-188+-+Add+new+metrics+to+support+health+checks
Thank you,
Rajini
Re: [VOTE] KIP-188 - Add new metrics to support health checks
Posted by Mickael Maison <mi...@gmail.com>.
+1 (non binding)
On Tue, Sep 5, 2017 at 6:51 PM, Jason Gustafson <ja...@confluent.io> wrote:
> +1 Lots of good stuff in here.
>
> One minor nit: in the name `FetchDownConversionsPerSec`, it's implicit that
> down-conversion is for messages. Could we do the same for
> `MessageConversionsTimeMs` and drop the `Message`? Then we don't have to
> decide if it should be 'Record' instead.
>
> On Tue, Sep 5, 2017 at 10:20 AM, Ismael Juma <is...@juma.me.uk> wrote:
>
>> Thanks Rajini.
>>
>> 1. I meant a topic metric, but we could have one for fetch and one for
>> produce differentiated by the additional tag. The advantage is that the
>> name would be consistent with the request metric for message conversions.
>> However, on closer inspection, this would make the name inconsistent with
>> the broker topic metrics:
>>
>> val totalProduceRequestRate =
>> newMeter(BrokerTopicStats.TotalProduceRequestsPerSec, "requests",
>> TimeUnit.SECONDS, tags)
>> val totalFetchRequestRate =
>> newMeter(BrokerTopicStats.TotalFetchRequestsPerSec, "requests",
>> TimeUnit.SECONDS, tags)
>>
>> So, we maybe we can instead go for FetchMessageConversionsPerSecond and
>> ProduceMessageConversionsPerSec.
>>
>> 2. Sounds good.
>>
>> Ismael
>>
>> On Tue, Sep 5, 2017 at 5:46 PM, Rajini Sivaram <ra...@gmail.com>
>> wrote:
>>
>> > Hi Ismael,
>> >
>> > 1. At the moment FetchDownConversionsPerSec is a topic metric while
>> > MessageConversionTimeMs is a request metric which indicates Produce/Fetch
>> > as a tag. Are you suggesting that we should convert
>> > FetchDownConversionsPerSec to a request metric called
>> > MessageConversionsPerSec
>> > for fetch requests?
>> >
>> > 2. TemporaryMessageSize for Produce/Fetch would indicate the space
>> > allocated for conversions. For other requests, this metric will not be
>> > created (unless we find a request where the size is large and this
>> > information is useful).
>> >
>> > Thank you,
>> >
>> > Rajini
>> >
>> >
>> > On Tue, Sep 5, 2017 at 4:55 PM, Ismael Juma <is...@juma.me.uk> wrote:
>> >
>> > > Thanks Rajini, +1 (binding) from me. Just a few minor comments:
>> > >
>> > > 1. FetchDownConversionsPerSec should probably be
>> MessageConversionsPerSec
>> > > with a request tag for consistency with MessageConversionsTimeMs. The
>> > text
>> > > in that paragraph should also be updated to talk about message
>> > conversions
>> > > instead of down conversions only.
>> > >
>> > > 2. What will TemporaryMemorySize represent for requests other than
>> > > `ProduceRequest`?
>> > >
>> > > Ismael
>> > >
>> > > On Mon, Sep 4, 2017 at 2:09 PM, Rajini Sivaram <
>> rajinisivaram@gmail.com>
>> > > wrote:
>> > >
>> > > > All the suggestions on the discuss thread have been incorporated into
>> > the
>> > > > KIP. Please let me know if you have any more concerns or else can we
>> > > > proceed with voting for this KIP?
>> > > >
>> > > > Thank you,
>> > > >
>> > > > Rajini
>> > > >
>> > > > On Thu, Aug 24, 2017 at 6:50 PM, Rajini Sivaram <
>> > rajinisivaram@gmail.com
>> > > >
>> > > > wrote:
>> > > >
>> > > > > Hi all,
>> > > > >
>> > > > > I would like to start the vote on KIP-188 that adds additional
>> > metrics
>> > > to
>> > > > > support health checks for Kafka Ops. Details are here:
>> > > > >
>> > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>> > > > > 188+-+Add+new+metrics+to+support+health+checks
>> > > > >
>> > > > > Thank you,
>> > > > >
>> > > > > Rajini
>> > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
Re: [VOTE] KIP-188 - Add new metrics to support health checks
Posted by Rajini Sivaram <ra...@gmail.com>.
Ok, thanks, leaving as is.
On Wed, Sep 6, 2017 at 1:22 AM, Jason Gustafson <ja...@confluent.io> wrote:
> >
> > I think I prefer the names with `Message` in them. For people less
> familiar
> > with Kafka, it makes it a bit clearer, I think.
>
>
> Works for me.
>
> On Tue, Sep 5, 2017 at 5:19 PM, Ismael Juma <is...@juma.me.uk> wrote:
>
> > I think I prefer the names with `Message` in them. For people less
> familiar
> > with Kafka, it makes it a bit clearer, I think.
> >
> > Ismael
> >
> > On Wed, Sep 6, 2017 at 12:39 AM, Rajini Sivaram <rajinisivaram@gmail.com
> >
> > wrote:
> >
> > > I am ok with dropping 'Message'. So the names would be
> > > FetchConversionsPerSec,
> > > ProduceConversionsPerSec and ConversionsTimeMs. The first two sound
> fine.
> > > Not so sure about ConversionsTimeMs, but since it appears with
> > > Produce/Fetch as the request tag, it should be ok. I haven't updated
> the
> > > KIP yet. If there are no objections, I will update the KIP tomorrow.
> > >
> > > Regards,
> > >
> > > Rajini
> > >
> > > On Tue, Sep 5, 2017 at 7:23 PM, Jason Gustafson <ja...@confluent.io>
> > > wrote:
> > >
> > > > >
> > > > > I was wondering about the message versus record question. The fact
> > that
> > > > we
> > > > > already have MessagesInPerSec seemed to favour the former. The
> other
> > > > aspect
> > > > > is that for produce requests, we can up convert as well, so it
> seemed
> > > > > better to keep it generic.
> > > >
> > > >
> > > > Yeah, so I thought maybe we could bypass the question and drop
> > `Message`
> > > > from the names if they were already clear enough. I'm fine with
> either
> > > way.
> > > >
> > > > On Tue, Sep 5, 2017 at 11:09 AM, Ismael Juma <is...@juma.me.uk>
> > wrote:
> > > >
> > > > > I was wondering about the message versus record question. The fact
> > that
> > > > we
> > > > > already have MessagesInPerSec seemed to favour the former. The
> other
> > > > aspect
> > > > > is that for produce requests, we can up convert as well, so it
> seemed
> > > > > better to keep it generic.
> > > > >
> > > > > Ismael
> > > > >
> > > > > On Tue, Sep 5, 2017 at 6:51 PM, Jason Gustafson <
> jason@confluent.io>
> > > > > wrote:
> > > > >
> > > > > > +1 Lots of good stuff in here.
> > > > > >
> > > > > > One minor nit: in the name `FetchDownConversionsPerSec`, it's
> > > implicit
> > > > > that
> > > > > > down-conversion is for messages. Could we do the same for
> > > > > > `MessageConversionsTimeMs` and drop the `Message`? Then we don't
> > have
> > > > to
> > > > > > decide if it should be 'Record' instead.
> > > > > >
> > > > > > On Tue, Sep 5, 2017 at 10:20 AM, Ismael Juma <is...@juma.me.uk>
> > > > wrote:
> > > > > >
> > > > > > > Thanks Rajini.
> > > > > > >
> > > > > > > 1. I meant a topic metric, but we could have one for fetch and
> > one
> > > > for
> > > > > > > produce differentiated by the additional tag. The advantage is
> > that
> > > > the
> > > > > > > name would be consistent with the request metric for message
> > > > > conversions.
> > > > > > > However, on closer inspection, this would make the name
> > > inconsistent
> > > > > with
> > > > > > > the broker topic metrics:
> > > > > > >
> > > > > > > val totalProduceRequestRate =
> > > > > > > newMeter(BrokerTopicStats.TotalProduceRequestsPerSec,
> > "requests",
> > > > > > > TimeUnit.SECONDS, tags)
> > > > > > > val totalFetchRequestRate =
> > > > > > > newMeter(BrokerTopicStats.TotalFetchRequestsPerSec,
> "requests",
> > > > > > > TimeUnit.SECONDS, tags)
> > > > > > >
> > > > > > > So, we maybe we can instead go for
> FetchMessageConversionsPerSeco
> > > nd
> > > > > and
> > > > > > > ProduceMessageConversionsPerSec.
> > > > > > >
> > > > > > > 2. Sounds good.
> > > > > > >
> > > > > > > Ismael
> > > > > > >
> > > > > > > On Tue, Sep 5, 2017 at 5:46 PM, Rajini Sivaram <
> > > > > rajinisivaram@gmail.com>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Hi Ismael,
> > > > > > > >
> > > > > > > > 1. At the moment FetchDownConversionsPerSec is a topic metric
> > > while
> > > > > > > > MessageConversionTimeMs is a request metric which indicates
> > > > > > Produce/Fetch
> > > > > > > > as a tag. Are you suggesting that we should convert
> > > > > > > > FetchDownConversionsPerSec to a request metric called
> > > > > > > > MessageConversionsPerSec
> > > > > > > > for fetch requests?
> > > > > > > >
> > > > > > > > 2. TemporaryMessageSize for Produce/Fetch would indicate the
> > > space
> > > > > > > > allocated for conversions. For other requests, this metric
> will
> > > not
> > > > > be
> > > > > > > > created (unless we find a request where the size is large and
> > > this
> > > > > > > > information is useful).
> > > > > > > >
> > > > > > > > Thank you,
> > > > > > > >
> > > > > > > > Rajini
> > > > > > > >
> > > > > > > >
> > > > > > > > On Tue, Sep 5, 2017 at 4:55 PM, Ismael Juma <
> ismael@juma.me.uk
> > >
> > > > > wrote:
> > > > > > > >
> > > > > > > > > Thanks Rajini, +1 (binding) from me. Just a few minor
> > comments:
> > > > > > > > >
> > > > > > > > > 1. FetchDownConversionsPerSec should probably be
> > > > > > > MessageConversionsPerSec
> > > > > > > > > with a request tag for consistency with
> > > MessageConversionsTimeMs.
> > > > > The
> > > > > > > > text
> > > > > > > > > in that paragraph should also be updated to talk about
> > message
> > > > > > > > conversions
> > > > > > > > > instead of down conversions only.
> > > > > > > > >
> > > > > > > > > 2. What will TemporaryMemorySize represent for requests
> other
> > > > than
> > > > > > > > > `ProduceRequest`?
> > > > > > > > >
> > > > > > > > > Ismael
> > > > > > > > >
> > > > > > > > > On Mon, Sep 4, 2017 at 2:09 PM, Rajini Sivaram <
> > > > > > > rajinisivaram@gmail.com>
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > All the suggestions on the discuss thread have been
> > > > incorporated
> > > > > > into
> > > > > > > > the
> > > > > > > > > > KIP. Please let me know if you have any more concerns or
> > else
> > > > can
> > > > > > we
> > > > > > > > > > proceed with voting for this KIP?
> > > > > > > > > >
> > > > > > > > > > Thank you,
> > > > > > > > > >
> > > > > > > > > > Rajini
> > > > > > > > > >
> > > > > > > > > > On Thu, Aug 24, 2017 at 6:50 PM, Rajini Sivaram <
> > > > > > > > rajinisivaram@gmail.com
> > > > > > > > > >
> > > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > > Hi all,
> > > > > > > > > > >
> > > > > > > > > > > I would like to start the vote on KIP-188 that adds
> > > > additional
> > > > > > > > metrics
> > > > > > > > > to
> > > > > > > > > > > support health checks for Kafka Ops. Details are here:
> > > > > > > > > > >
> > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > > > > > > > 188+-+Add+new+metrics+to+support+health+checks
> > > > > > > > > > >
> > > > > > > > > > > Thank you,
> > > > > > > > > > >
> > > > > > > > > > > Rajini
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [VOTE] KIP-188 - Add new metrics to support health checks
Posted by Jason Gustafson <ja...@confluent.io>.
>
> I think I prefer the names with `Message` in them. For people less familiar
> with Kafka, it makes it a bit clearer, I think.
Works for me.
On Tue, Sep 5, 2017 at 5:19 PM, Ismael Juma <is...@juma.me.uk> wrote:
> I think I prefer the names with `Message` in them. For people less familiar
> with Kafka, it makes it a bit clearer, I think.
>
> Ismael
>
> On Wed, Sep 6, 2017 at 12:39 AM, Rajini Sivaram <ra...@gmail.com>
> wrote:
>
> > I am ok with dropping 'Message'. So the names would be
> > FetchConversionsPerSec,
> > ProduceConversionsPerSec and ConversionsTimeMs. The first two sound fine.
> > Not so sure about ConversionsTimeMs, but since it appears with
> > Produce/Fetch as the request tag, it should be ok. I haven't updated the
> > KIP yet. If there are no objections, I will update the KIP tomorrow.
> >
> > Regards,
> >
> > Rajini
> >
> > On Tue, Sep 5, 2017 at 7:23 PM, Jason Gustafson <ja...@confluent.io>
> > wrote:
> >
> > > >
> > > > I was wondering about the message versus record question. The fact
> that
> > > we
> > > > already have MessagesInPerSec seemed to favour the former. The other
> > > aspect
> > > > is that for produce requests, we can up convert as well, so it seemed
> > > > better to keep it generic.
> > >
> > >
> > > Yeah, so I thought maybe we could bypass the question and drop
> `Message`
> > > from the names if they were already clear enough. I'm fine with either
> > way.
> > >
> > > On Tue, Sep 5, 2017 at 11:09 AM, Ismael Juma <is...@juma.me.uk>
> wrote:
> > >
> > > > I was wondering about the message versus record question. The fact
> that
> > > we
> > > > already have MessagesInPerSec seemed to favour the former. The other
> > > aspect
> > > > is that for produce requests, we can up convert as well, so it seemed
> > > > better to keep it generic.
> > > >
> > > > Ismael
> > > >
> > > > On Tue, Sep 5, 2017 at 6:51 PM, Jason Gustafson <ja...@confluent.io>
> > > > wrote:
> > > >
> > > > > +1 Lots of good stuff in here.
> > > > >
> > > > > One minor nit: in the name `FetchDownConversionsPerSec`, it's
> > implicit
> > > > that
> > > > > down-conversion is for messages. Could we do the same for
> > > > > `MessageConversionsTimeMs` and drop the `Message`? Then we don't
> have
> > > to
> > > > > decide if it should be 'Record' instead.
> > > > >
> > > > > On Tue, Sep 5, 2017 at 10:20 AM, Ismael Juma <is...@juma.me.uk>
> > > wrote:
> > > > >
> > > > > > Thanks Rajini.
> > > > > >
> > > > > > 1. I meant a topic metric, but we could have one for fetch and
> one
> > > for
> > > > > > produce differentiated by the additional tag. The advantage is
> that
> > > the
> > > > > > name would be consistent with the request metric for message
> > > > conversions.
> > > > > > However, on closer inspection, this would make the name
> > inconsistent
> > > > with
> > > > > > the broker topic metrics:
> > > > > >
> > > > > > val totalProduceRequestRate =
> > > > > > newMeter(BrokerTopicStats.TotalProduceRequestsPerSec,
> "requests",
> > > > > > TimeUnit.SECONDS, tags)
> > > > > > val totalFetchRequestRate =
> > > > > > newMeter(BrokerTopicStats.TotalFetchRequestsPerSec, "requests",
> > > > > > TimeUnit.SECONDS, tags)
> > > > > >
> > > > > > So, we maybe we can instead go for FetchMessageConversionsPerSeco
> > nd
> > > > and
> > > > > > ProduceMessageConversionsPerSec.
> > > > > >
> > > > > > 2. Sounds good.
> > > > > >
> > > > > > Ismael
> > > > > >
> > > > > > On Tue, Sep 5, 2017 at 5:46 PM, Rajini Sivaram <
> > > > rajinisivaram@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > > > Hi Ismael,
> > > > > > >
> > > > > > > 1. At the moment FetchDownConversionsPerSec is a topic metric
> > while
> > > > > > > MessageConversionTimeMs is a request metric which indicates
> > > > > Produce/Fetch
> > > > > > > as a tag. Are you suggesting that we should convert
> > > > > > > FetchDownConversionsPerSec to a request metric called
> > > > > > > MessageConversionsPerSec
> > > > > > > for fetch requests?
> > > > > > >
> > > > > > > 2. TemporaryMessageSize for Produce/Fetch would indicate the
> > space
> > > > > > > allocated for conversions. For other requests, this metric will
> > not
> > > > be
> > > > > > > created (unless we find a request where the size is large and
> > this
> > > > > > > information is useful).
> > > > > > >
> > > > > > > Thank you,
> > > > > > >
> > > > > > > Rajini
> > > > > > >
> > > > > > >
> > > > > > > On Tue, Sep 5, 2017 at 4:55 PM, Ismael Juma <ismael@juma.me.uk
> >
> > > > wrote:
> > > > > > >
> > > > > > > > Thanks Rajini, +1 (binding) from me. Just a few minor
> comments:
> > > > > > > >
> > > > > > > > 1. FetchDownConversionsPerSec should probably be
> > > > > > MessageConversionsPerSec
> > > > > > > > with a request tag for consistency with
> > MessageConversionsTimeMs.
> > > > The
> > > > > > > text
> > > > > > > > in that paragraph should also be updated to talk about
> message
> > > > > > > conversions
> > > > > > > > instead of down conversions only.
> > > > > > > >
> > > > > > > > 2. What will TemporaryMemorySize represent for requests other
> > > than
> > > > > > > > `ProduceRequest`?
> > > > > > > >
> > > > > > > > Ismael
> > > > > > > >
> > > > > > > > On Mon, Sep 4, 2017 at 2:09 PM, Rajini Sivaram <
> > > > > > rajinisivaram@gmail.com>
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > All the suggestions on the discuss thread have been
> > > incorporated
> > > > > into
> > > > > > > the
> > > > > > > > > KIP. Please let me know if you have any more concerns or
> else
> > > can
> > > > > we
> > > > > > > > > proceed with voting for this KIP?
> > > > > > > > >
> > > > > > > > > Thank you,
> > > > > > > > >
> > > > > > > > > Rajini
> > > > > > > > >
> > > > > > > > > On Thu, Aug 24, 2017 at 6:50 PM, Rajini Sivaram <
> > > > > > > rajinisivaram@gmail.com
> > > > > > > > >
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Hi all,
> > > > > > > > > >
> > > > > > > > > > I would like to start the vote on KIP-188 that adds
> > > additional
> > > > > > > metrics
> > > > > > > > to
> > > > > > > > > > support health checks for Kafka Ops. Details are here:
> > > > > > > > > >
> > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > > > > > > 188+-+Add+new+metrics+to+support+health+checks
> > > > > > > > > >
> > > > > > > > > > Thank you,
> > > > > > > > > >
> > > > > > > > > > Rajini
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [VOTE] KIP-188 - Add new metrics to support health checks
Posted by Ismael Juma <is...@juma.me.uk>.
I think I prefer the names with `Message` in them. For people less familiar
with Kafka, it makes it a bit clearer, I think.
Ismael
On Wed, Sep 6, 2017 at 12:39 AM, Rajini Sivaram <ra...@gmail.com>
wrote:
> I am ok with dropping 'Message'. So the names would be
> FetchConversionsPerSec,
> ProduceConversionsPerSec and ConversionsTimeMs. The first two sound fine.
> Not so sure about ConversionsTimeMs, but since it appears with
> Produce/Fetch as the request tag, it should be ok. I haven't updated the
> KIP yet. If there are no objections, I will update the KIP tomorrow.
>
> Regards,
>
> Rajini
>
> On Tue, Sep 5, 2017 at 7:23 PM, Jason Gustafson <ja...@confluent.io>
> wrote:
>
> > >
> > > I was wondering about the message versus record question. The fact that
> > we
> > > already have MessagesInPerSec seemed to favour the former. The other
> > aspect
> > > is that for produce requests, we can up convert as well, so it seemed
> > > better to keep it generic.
> >
> >
> > Yeah, so I thought maybe we could bypass the question and drop `Message`
> > from the names if they were already clear enough. I'm fine with either
> way.
> >
> > On Tue, Sep 5, 2017 at 11:09 AM, Ismael Juma <is...@juma.me.uk> wrote:
> >
> > > I was wondering about the message versus record question. The fact that
> > we
> > > already have MessagesInPerSec seemed to favour the former. The other
> > aspect
> > > is that for produce requests, we can up convert as well, so it seemed
> > > better to keep it generic.
> > >
> > > Ismael
> > >
> > > On Tue, Sep 5, 2017 at 6:51 PM, Jason Gustafson <ja...@confluent.io>
> > > wrote:
> > >
> > > > +1 Lots of good stuff in here.
> > > >
> > > > One minor nit: in the name `FetchDownConversionsPerSec`, it's
> implicit
> > > that
> > > > down-conversion is for messages. Could we do the same for
> > > > `MessageConversionsTimeMs` and drop the `Message`? Then we don't have
> > to
> > > > decide if it should be 'Record' instead.
> > > >
> > > > On Tue, Sep 5, 2017 at 10:20 AM, Ismael Juma <is...@juma.me.uk>
> > wrote:
> > > >
> > > > > Thanks Rajini.
> > > > >
> > > > > 1. I meant a topic metric, but we could have one for fetch and one
> > for
> > > > > produce differentiated by the additional tag. The advantage is that
> > the
> > > > > name would be consistent with the request metric for message
> > > conversions.
> > > > > However, on closer inspection, this would make the name
> inconsistent
> > > with
> > > > > the broker topic metrics:
> > > > >
> > > > > val totalProduceRequestRate =
> > > > > newMeter(BrokerTopicStats.TotalProduceRequestsPerSec, "requests",
> > > > > TimeUnit.SECONDS, tags)
> > > > > val totalFetchRequestRate =
> > > > > newMeter(BrokerTopicStats.TotalFetchRequestsPerSec, "requests",
> > > > > TimeUnit.SECONDS, tags)
> > > > >
> > > > > So, we maybe we can instead go for FetchMessageConversionsPerSeco
> nd
> > > and
> > > > > ProduceMessageConversionsPerSec.
> > > > >
> > > > > 2. Sounds good.
> > > > >
> > > > > Ismael
> > > > >
> > > > > On Tue, Sep 5, 2017 at 5:46 PM, Rajini Sivaram <
> > > rajinisivaram@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Hi Ismael,
> > > > > >
> > > > > > 1. At the moment FetchDownConversionsPerSec is a topic metric
> while
> > > > > > MessageConversionTimeMs is a request metric which indicates
> > > > Produce/Fetch
> > > > > > as a tag. Are you suggesting that we should convert
> > > > > > FetchDownConversionsPerSec to a request metric called
> > > > > > MessageConversionsPerSec
> > > > > > for fetch requests?
> > > > > >
> > > > > > 2. TemporaryMessageSize for Produce/Fetch would indicate the
> space
> > > > > > allocated for conversions. For other requests, this metric will
> not
> > > be
> > > > > > created (unless we find a request where the size is large and
> this
> > > > > > information is useful).
> > > > > >
> > > > > > Thank you,
> > > > > >
> > > > > > Rajini
> > > > > >
> > > > > >
> > > > > > On Tue, Sep 5, 2017 at 4:55 PM, Ismael Juma <is...@juma.me.uk>
> > > wrote:
> > > > > >
> > > > > > > Thanks Rajini, +1 (binding) from me. Just a few minor comments:
> > > > > > >
> > > > > > > 1. FetchDownConversionsPerSec should probably be
> > > > > MessageConversionsPerSec
> > > > > > > with a request tag for consistency with
> MessageConversionsTimeMs.
> > > The
> > > > > > text
> > > > > > > in that paragraph should also be updated to talk about message
> > > > > > conversions
> > > > > > > instead of down conversions only.
> > > > > > >
> > > > > > > 2. What will TemporaryMemorySize represent for requests other
> > than
> > > > > > > `ProduceRequest`?
> > > > > > >
> > > > > > > Ismael
> > > > > > >
> > > > > > > On Mon, Sep 4, 2017 at 2:09 PM, Rajini Sivaram <
> > > > > rajinisivaram@gmail.com>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > All the suggestions on the discuss thread have been
> > incorporated
> > > > into
> > > > > > the
> > > > > > > > KIP. Please let me know if you have any more concerns or else
> > can
> > > > we
> > > > > > > > proceed with voting for this KIP?
> > > > > > > >
> > > > > > > > Thank you,
> > > > > > > >
> > > > > > > > Rajini
> > > > > > > >
> > > > > > > > On Thu, Aug 24, 2017 at 6:50 PM, Rajini Sivaram <
> > > > > > rajinisivaram@gmail.com
> > > > > > > >
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Hi all,
> > > > > > > > >
> > > > > > > > > I would like to start the vote on KIP-188 that adds
> > additional
> > > > > > metrics
> > > > > > > to
> > > > > > > > > support health checks for Kafka Ops. Details are here:
> > > > > > > > >
> > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > > > > > 188+-+Add+new+metrics+to+support+health+checks
> > > > > > > > >
> > > > > > > > > Thank you,
> > > > > > > > >
> > > > > > > > > Rajini
> > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [VOTE] KIP-188 - Add new metrics to support health checks
Posted by Rajini Sivaram <ra...@gmail.com>.
I am ok with dropping 'Message'. So the names would be FetchConversionsPerSec,
ProduceConversionsPerSec and ConversionsTimeMs. The first two sound fine.
Not so sure about ConversionsTimeMs, but since it appears with
Produce/Fetch as the request tag, it should be ok. I haven't updated the
KIP yet. If there are no objections, I will update the KIP tomorrow.
Regards,
Rajini
On Tue, Sep 5, 2017 at 7:23 PM, Jason Gustafson <ja...@confluent.io> wrote:
> >
> > I was wondering about the message versus record question. The fact that
> we
> > already have MessagesInPerSec seemed to favour the former. The other
> aspect
> > is that for produce requests, we can up convert as well, so it seemed
> > better to keep it generic.
>
>
> Yeah, so I thought maybe we could bypass the question and drop `Message`
> from the names if they were already clear enough. I'm fine with either way.
>
> On Tue, Sep 5, 2017 at 11:09 AM, Ismael Juma <is...@juma.me.uk> wrote:
>
> > I was wondering about the message versus record question. The fact that
> we
> > already have MessagesInPerSec seemed to favour the former. The other
> aspect
> > is that for produce requests, we can up convert as well, so it seemed
> > better to keep it generic.
> >
> > Ismael
> >
> > On Tue, Sep 5, 2017 at 6:51 PM, Jason Gustafson <ja...@confluent.io>
> > wrote:
> >
> > > +1 Lots of good stuff in here.
> > >
> > > One minor nit: in the name `FetchDownConversionsPerSec`, it's implicit
> > that
> > > down-conversion is for messages. Could we do the same for
> > > `MessageConversionsTimeMs` and drop the `Message`? Then we don't have
> to
> > > decide if it should be 'Record' instead.
> > >
> > > On Tue, Sep 5, 2017 at 10:20 AM, Ismael Juma <is...@juma.me.uk>
> wrote:
> > >
> > > > Thanks Rajini.
> > > >
> > > > 1. I meant a topic metric, but we could have one for fetch and one
> for
> > > > produce differentiated by the additional tag. The advantage is that
> the
> > > > name would be consistent with the request metric for message
> > conversions.
> > > > However, on closer inspection, this would make the name inconsistent
> > with
> > > > the broker topic metrics:
> > > >
> > > > val totalProduceRequestRate =
> > > > newMeter(BrokerTopicStats.TotalProduceRequestsPerSec, "requests",
> > > > TimeUnit.SECONDS, tags)
> > > > val totalFetchRequestRate =
> > > > newMeter(BrokerTopicStats.TotalFetchRequestsPerSec, "requests",
> > > > TimeUnit.SECONDS, tags)
> > > >
> > > > So, we maybe we can instead go for FetchMessageConversionsPerSecond
> > and
> > > > ProduceMessageConversionsPerSec.
> > > >
> > > > 2. Sounds good.
> > > >
> > > > Ismael
> > > >
> > > > On Tue, Sep 5, 2017 at 5:46 PM, Rajini Sivaram <
> > rajinisivaram@gmail.com>
> > > > wrote:
> > > >
> > > > > Hi Ismael,
> > > > >
> > > > > 1. At the moment FetchDownConversionsPerSec is a topic metric while
> > > > > MessageConversionTimeMs is a request metric which indicates
> > > Produce/Fetch
> > > > > as a tag. Are you suggesting that we should convert
> > > > > FetchDownConversionsPerSec to a request metric called
> > > > > MessageConversionsPerSec
> > > > > for fetch requests?
> > > > >
> > > > > 2. TemporaryMessageSize for Produce/Fetch would indicate the space
> > > > > allocated for conversions. For other requests, this metric will not
> > be
> > > > > created (unless we find a request where the size is large and this
> > > > > information is useful).
> > > > >
> > > > > Thank you,
> > > > >
> > > > > Rajini
> > > > >
> > > > >
> > > > > On Tue, Sep 5, 2017 at 4:55 PM, Ismael Juma <is...@juma.me.uk>
> > wrote:
> > > > >
> > > > > > Thanks Rajini, +1 (binding) from me. Just a few minor comments:
> > > > > >
> > > > > > 1. FetchDownConversionsPerSec should probably be
> > > > MessageConversionsPerSec
> > > > > > with a request tag for consistency with MessageConversionsTimeMs.
> > The
> > > > > text
> > > > > > in that paragraph should also be updated to talk about message
> > > > > conversions
> > > > > > instead of down conversions only.
> > > > > >
> > > > > > 2. What will TemporaryMemorySize represent for requests other
> than
> > > > > > `ProduceRequest`?
> > > > > >
> > > > > > Ismael
> > > > > >
> > > > > > On Mon, Sep 4, 2017 at 2:09 PM, Rajini Sivaram <
> > > > rajinisivaram@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > > > All the suggestions on the discuss thread have been
> incorporated
> > > into
> > > > > the
> > > > > > > KIP. Please let me know if you have any more concerns or else
> can
> > > we
> > > > > > > proceed with voting for this KIP?
> > > > > > >
> > > > > > > Thank you,
> > > > > > >
> > > > > > > Rajini
> > > > > > >
> > > > > > > On Thu, Aug 24, 2017 at 6:50 PM, Rajini Sivaram <
> > > > > rajinisivaram@gmail.com
> > > > > > >
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Hi all,
> > > > > > > >
> > > > > > > > I would like to start the vote on KIP-188 that adds
> additional
> > > > > metrics
> > > > > > to
> > > > > > > > support health checks for Kafka Ops. Details are here:
> > > > > > > >
> > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > > > > 188+-+Add+new+metrics+to+support+health+checks
> > > > > > > >
> > > > > > > > Thank you,
> > > > > > > >
> > > > > > > > Rajini
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [VOTE] KIP-188 - Add new metrics to support health checks
Posted by Jason Gustafson <ja...@confluent.io>.
>
> I was wondering about the message versus record question. The fact that we
> already have MessagesInPerSec seemed to favour the former. The other aspect
> is that for produce requests, we can up convert as well, so it seemed
> better to keep it generic.
Yeah, so I thought maybe we could bypass the question and drop `Message`
from the names if they were already clear enough. I'm fine with either way.
On Tue, Sep 5, 2017 at 11:09 AM, Ismael Juma <is...@juma.me.uk> wrote:
> I was wondering about the message versus record question. The fact that we
> already have MessagesInPerSec seemed to favour the former. The other aspect
> is that for produce requests, we can up convert as well, so it seemed
> better to keep it generic.
>
> Ismael
>
> On Tue, Sep 5, 2017 at 6:51 PM, Jason Gustafson <ja...@confluent.io>
> wrote:
>
> > +1 Lots of good stuff in here.
> >
> > One minor nit: in the name `FetchDownConversionsPerSec`, it's implicit
> that
> > down-conversion is for messages. Could we do the same for
> > `MessageConversionsTimeMs` and drop the `Message`? Then we don't have to
> > decide if it should be 'Record' instead.
> >
> > On Tue, Sep 5, 2017 at 10:20 AM, Ismael Juma <is...@juma.me.uk> wrote:
> >
> > > Thanks Rajini.
> > >
> > > 1. I meant a topic metric, but we could have one for fetch and one for
> > > produce differentiated by the additional tag. The advantage is that the
> > > name would be consistent with the request metric for message
> conversions.
> > > However, on closer inspection, this would make the name inconsistent
> with
> > > the broker topic metrics:
> > >
> > > val totalProduceRequestRate =
> > > newMeter(BrokerTopicStats.TotalProduceRequestsPerSec, "requests",
> > > TimeUnit.SECONDS, tags)
> > > val totalFetchRequestRate =
> > > newMeter(BrokerTopicStats.TotalFetchRequestsPerSec, "requests",
> > > TimeUnit.SECONDS, tags)
> > >
> > > So, we maybe we can instead go for FetchMessageConversionsPerSecond
> and
> > > ProduceMessageConversionsPerSec.
> > >
> > > 2. Sounds good.
> > >
> > > Ismael
> > >
> > > On Tue, Sep 5, 2017 at 5:46 PM, Rajini Sivaram <
> rajinisivaram@gmail.com>
> > > wrote:
> > >
> > > > Hi Ismael,
> > > >
> > > > 1. At the moment FetchDownConversionsPerSec is a topic metric while
> > > > MessageConversionTimeMs is a request metric which indicates
> > Produce/Fetch
> > > > as a tag. Are you suggesting that we should convert
> > > > FetchDownConversionsPerSec to a request metric called
> > > > MessageConversionsPerSec
> > > > for fetch requests?
> > > >
> > > > 2. TemporaryMessageSize for Produce/Fetch would indicate the space
> > > > allocated for conversions. For other requests, this metric will not
> be
> > > > created (unless we find a request where the size is large and this
> > > > information is useful).
> > > >
> > > > Thank you,
> > > >
> > > > Rajini
> > > >
> > > >
> > > > On Tue, Sep 5, 2017 at 4:55 PM, Ismael Juma <is...@juma.me.uk>
> wrote:
> > > >
> > > > > Thanks Rajini, +1 (binding) from me. Just a few minor comments:
> > > > >
> > > > > 1. FetchDownConversionsPerSec should probably be
> > > MessageConversionsPerSec
> > > > > with a request tag for consistency with MessageConversionsTimeMs.
> The
> > > > text
> > > > > in that paragraph should also be updated to talk about message
> > > > conversions
> > > > > instead of down conversions only.
> > > > >
> > > > > 2. What will TemporaryMemorySize represent for requests other than
> > > > > `ProduceRequest`?
> > > > >
> > > > > Ismael
> > > > >
> > > > > On Mon, Sep 4, 2017 at 2:09 PM, Rajini Sivaram <
> > > rajinisivaram@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > All the suggestions on the discuss thread have been incorporated
> > into
> > > > the
> > > > > > KIP. Please let me know if you have any more concerns or else can
> > we
> > > > > > proceed with voting for this KIP?
> > > > > >
> > > > > > Thank you,
> > > > > >
> > > > > > Rajini
> > > > > >
> > > > > > On Thu, Aug 24, 2017 at 6:50 PM, Rajini Sivaram <
> > > > rajinisivaram@gmail.com
> > > > > >
> > > > > > wrote:
> > > > > >
> > > > > > > Hi all,
> > > > > > >
> > > > > > > I would like to start the vote on KIP-188 that adds additional
> > > > metrics
> > > > > to
> > > > > > > support health checks for Kafka Ops. Details are here:
> > > > > > >
> > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > > > 188+-+Add+new+metrics+to+support+health+checks
> > > > > > >
> > > > > > > Thank you,
> > > > > > >
> > > > > > > Rajini
> > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [VOTE] KIP-188 - Add new metrics to support health checks
Posted by Ismael Juma <is...@juma.me.uk>.
I was wondering about the message versus record question. The fact that we
already have MessagesInPerSec seemed to favour the former. The other aspect
is that for produce requests, we can up convert as well, so it seemed
better to keep it generic.
Ismael
On Tue, Sep 5, 2017 at 6:51 PM, Jason Gustafson <ja...@confluent.io> wrote:
> +1 Lots of good stuff in here.
>
> One minor nit: in the name `FetchDownConversionsPerSec`, it's implicit that
> down-conversion is for messages. Could we do the same for
> `MessageConversionsTimeMs` and drop the `Message`? Then we don't have to
> decide if it should be 'Record' instead.
>
> On Tue, Sep 5, 2017 at 10:20 AM, Ismael Juma <is...@juma.me.uk> wrote:
>
> > Thanks Rajini.
> >
> > 1. I meant a topic metric, but we could have one for fetch and one for
> > produce differentiated by the additional tag. The advantage is that the
> > name would be consistent with the request metric for message conversions.
> > However, on closer inspection, this would make the name inconsistent with
> > the broker topic metrics:
> >
> > val totalProduceRequestRate =
> > newMeter(BrokerTopicStats.TotalProduceRequestsPerSec, "requests",
> > TimeUnit.SECONDS, tags)
> > val totalFetchRequestRate =
> > newMeter(BrokerTopicStats.TotalFetchRequestsPerSec, "requests",
> > TimeUnit.SECONDS, tags)
> >
> > So, we maybe we can instead go for FetchMessageConversionsPerSecond and
> > ProduceMessageConversionsPerSec.
> >
> > 2. Sounds good.
> >
> > Ismael
> >
> > On Tue, Sep 5, 2017 at 5:46 PM, Rajini Sivaram <ra...@gmail.com>
> > wrote:
> >
> > > Hi Ismael,
> > >
> > > 1. At the moment FetchDownConversionsPerSec is a topic metric while
> > > MessageConversionTimeMs is a request metric which indicates
> Produce/Fetch
> > > as a tag. Are you suggesting that we should convert
> > > FetchDownConversionsPerSec to a request metric called
> > > MessageConversionsPerSec
> > > for fetch requests?
> > >
> > > 2. TemporaryMessageSize for Produce/Fetch would indicate the space
> > > allocated for conversions. For other requests, this metric will not be
> > > created (unless we find a request where the size is large and this
> > > information is useful).
> > >
> > > Thank you,
> > >
> > > Rajini
> > >
> > >
> > > On Tue, Sep 5, 2017 at 4:55 PM, Ismael Juma <is...@juma.me.uk> wrote:
> > >
> > > > Thanks Rajini, +1 (binding) from me. Just a few minor comments:
> > > >
> > > > 1. FetchDownConversionsPerSec should probably be
> > MessageConversionsPerSec
> > > > with a request tag for consistency with MessageConversionsTimeMs. The
> > > text
> > > > in that paragraph should also be updated to talk about message
> > > conversions
> > > > instead of down conversions only.
> > > >
> > > > 2. What will TemporaryMemorySize represent for requests other than
> > > > `ProduceRequest`?
> > > >
> > > > Ismael
> > > >
> > > > On Mon, Sep 4, 2017 at 2:09 PM, Rajini Sivaram <
> > rajinisivaram@gmail.com>
> > > > wrote:
> > > >
> > > > > All the suggestions on the discuss thread have been incorporated
> into
> > > the
> > > > > KIP. Please let me know if you have any more concerns or else can
> we
> > > > > proceed with voting for this KIP?
> > > > >
> > > > > Thank you,
> > > > >
> > > > > Rajini
> > > > >
> > > > > On Thu, Aug 24, 2017 at 6:50 PM, Rajini Sivaram <
> > > rajinisivaram@gmail.com
> > > > >
> > > > > wrote:
> > > > >
> > > > > > Hi all,
> > > > > >
> > > > > > I would like to start the vote on KIP-188 that adds additional
> > > metrics
> > > > to
> > > > > > support health checks for Kafka Ops. Details are here:
> > > > > >
> > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > > 188+-+Add+new+metrics+to+support+health+checks
> > > > > >
> > > > > > Thank you,
> > > > > >
> > > > > > Rajini
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [VOTE] KIP-188 - Add new metrics to support health checks
Posted by Jason Gustafson <ja...@confluent.io>.
+1 Lots of good stuff in here.
One minor nit: in the name `FetchDownConversionsPerSec`, it's implicit that
down-conversion is for messages. Could we do the same for
`MessageConversionsTimeMs` and drop the `Message`? Then we don't have to
decide if it should be 'Record' instead.
On Tue, Sep 5, 2017 at 10:20 AM, Ismael Juma <is...@juma.me.uk> wrote:
> Thanks Rajini.
>
> 1. I meant a topic metric, but we could have one for fetch and one for
> produce differentiated by the additional tag. The advantage is that the
> name would be consistent with the request metric for message conversions.
> However, on closer inspection, this would make the name inconsistent with
> the broker topic metrics:
>
> val totalProduceRequestRate =
> newMeter(BrokerTopicStats.TotalProduceRequestsPerSec, "requests",
> TimeUnit.SECONDS, tags)
> val totalFetchRequestRate =
> newMeter(BrokerTopicStats.TotalFetchRequestsPerSec, "requests",
> TimeUnit.SECONDS, tags)
>
> So, we maybe we can instead go for FetchMessageConversionsPerSecond and
> ProduceMessageConversionsPerSec.
>
> 2. Sounds good.
>
> Ismael
>
> On Tue, Sep 5, 2017 at 5:46 PM, Rajini Sivaram <ra...@gmail.com>
> wrote:
>
> > Hi Ismael,
> >
> > 1. At the moment FetchDownConversionsPerSec is a topic metric while
> > MessageConversionTimeMs is a request metric which indicates Produce/Fetch
> > as a tag. Are you suggesting that we should convert
> > FetchDownConversionsPerSec to a request metric called
> > MessageConversionsPerSec
> > for fetch requests?
> >
> > 2. TemporaryMessageSize for Produce/Fetch would indicate the space
> > allocated for conversions. For other requests, this metric will not be
> > created (unless we find a request where the size is large and this
> > information is useful).
> >
> > Thank you,
> >
> > Rajini
> >
> >
> > On Tue, Sep 5, 2017 at 4:55 PM, Ismael Juma <is...@juma.me.uk> wrote:
> >
> > > Thanks Rajini, +1 (binding) from me. Just a few minor comments:
> > >
> > > 1. FetchDownConversionsPerSec should probably be
> MessageConversionsPerSec
> > > with a request tag for consistency with MessageConversionsTimeMs. The
> > text
> > > in that paragraph should also be updated to talk about message
> > conversions
> > > instead of down conversions only.
> > >
> > > 2. What will TemporaryMemorySize represent for requests other than
> > > `ProduceRequest`?
> > >
> > > Ismael
> > >
> > > On Mon, Sep 4, 2017 at 2:09 PM, Rajini Sivaram <
> rajinisivaram@gmail.com>
> > > wrote:
> > >
> > > > All the suggestions on the discuss thread have been incorporated into
> > the
> > > > KIP. Please let me know if you have any more concerns or else can we
> > > > proceed with voting for this KIP?
> > > >
> > > > Thank you,
> > > >
> > > > Rajini
> > > >
> > > > On Thu, Aug 24, 2017 at 6:50 PM, Rajini Sivaram <
> > rajinisivaram@gmail.com
> > > >
> > > > wrote:
> > > >
> > > > > Hi all,
> > > > >
> > > > > I would like to start the vote on KIP-188 that adds additional
> > metrics
> > > to
> > > > > support health checks for Kafka Ops. Details are here:
> > > > >
> > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > 188+-+Add+new+metrics+to+support+health+checks
> > > > >
> > > > > Thank you,
> > > > >
> > > > > Rajini
> > > > >
> > > > >
> > > >
> > >
> >
>
Re: [VOTE] KIP-188 - Add new metrics to support health checks
Posted by Rajini Sivaram <ra...@gmail.com>.
Hi Ismael,
1. Yes, that makes sense. Updated the KIP to use FetchMessageConversionsPerSec
and ProduceMessageConversionsPerSec.
Thank you,
Rajini
On Tue, Sep 5, 2017 at 6:20 PM, Ismael Juma <is...@juma.me.uk> wrote:
> Thanks Rajini.
>
> 1. I meant a topic metric, but we could have one for fetch and one for
> produce differentiated by the additional tag. The advantage is that the
> name would be consistent with the request metric for message conversions.
> However, on closer inspection, this would make the name inconsistent with
> the broker topic metrics:
>
> val totalProduceRequestRate =
> newMeter(BrokerTopicStats.TotalProduceRequestsPerSec, "requests",
> TimeUnit.SECONDS, tags)
> val totalFetchRequestRate =
> newMeter(BrokerTopicStats.TotalFetchRequestsPerSec, "requests",
> TimeUnit.SECONDS, tags)
>
> So, we maybe we can instead go for FetchMessageConversionsPerSecond and
> ProduceMessageConversionsPerSec.
>
> 2. Sounds good.
>
> Ismael
>
> On Tue, Sep 5, 2017 at 5:46 PM, Rajini Sivaram <ra...@gmail.com>
> wrote:
>
> > Hi Ismael,
> >
> > 1. At the moment FetchDownConversionsPerSec is a topic metric while
> > MessageConversionTimeMs is a request metric which indicates Produce/Fetch
> > as a tag. Are you suggesting that we should convert
> > FetchDownConversionsPerSec to a request metric called
> > MessageConversionsPerSec
> > for fetch requests?
> >
> > 2. TemporaryMessageSize for Produce/Fetch would indicate the space
> > allocated for conversions. For other requests, this metric will not be
> > created (unless we find a request where the size is large and this
> > information is useful).
> >
> > Thank you,
> >
> > Rajini
> >
> >
> > On Tue, Sep 5, 2017 at 4:55 PM, Ismael Juma <is...@juma.me.uk> wrote:
> >
> > > Thanks Rajini, +1 (binding) from me. Just a few minor comments:
> > >
> > > 1. FetchDownConversionsPerSec should probably be
> MessageConversionsPerSec
> > > with a request tag for consistency with MessageConversionsTimeMs. The
> > text
> > > in that paragraph should also be updated to talk about message
> > conversions
> > > instead of down conversions only.
> > >
> > > 2. What will TemporaryMemorySize represent for requests other than
> > > `ProduceRequest`?
> > >
> > > Ismael
> > >
> > > On Mon, Sep 4, 2017 at 2:09 PM, Rajini Sivaram <
> rajinisivaram@gmail.com>
> > > wrote:
> > >
> > > > All the suggestions on the discuss thread have been incorporated into
> > the
> > > > KIP. Please let me know if you have any more concerns or else can we
> > > > proceed with voting for this KIP?
> > > >
> > > > Thank you,
> > > >
> > > > Rajini
> > > >
> > > > On Thu, Aug 24, 2017 at 6:50 PM, Rajini Sivaram <
> > rajinisivaram@gmail.com
> > > >
> > > > wrote:
> > > >
> > > > > Hi all,
> > > > >
> > > > > I would like to start the vote on KIP-188 that adds additional
> > metrics
> > > to
> > > > > support health checks for Kafka Ops. Details are here:
> > > > >
> > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > 188+-+Add+new+metrics+to+support+health+checks
> > > > >
> > > > > Thank you,
> > > > >
> > > > > Rajini
> > > > >
> > > > >
> > > >
> > >
> >
>
Re: [VOTE] KIP-188 - Add new metrics to support health checks
Posted by Ismael Juma <is...@juma.me.uk>.
Thanks Rajini.
1. I meant a topic metric, but we could have one for fetch and one for
produce differentiated by the additional tag. The advantage is that the
name would be consistent with the request metric for message conversions.
However, on closer inspection, this would make the name inconsistent with
the broker topic metrics:
val totalProduceRequestRate =
newMeter(BrokerTopicStats.TotalProduceRequestsPerSec, "requests",
TimeUnit.SECONDS, tags)
val totalFetchRequestRate =
newMeter(BrokerTopicStats.TotalFetchRequestsPerSec, "requests",
TimeUnit.SECONDS, tags)
So, we maybe we can instead go for FetchMessageConversionsPerSecond and
ProduceMessageConversionsPerSec.
2. Sounds good.
Ismael
On Tue, Sep 5, 2017 at 5:46 PM, Rajini Sivaram <ra...@gmail.com>
wrote:
> Hi Ismael,
>
> 1. At the moment FetchDownConversionsPerSec is a topic metric while
> MessageConversionTimeMs is a request metric which indicates Produce/Fetch
> as a tag. Are you suggesting that we should convert
> FetchDownConversionsPerSec to a request metric called
> MessageConversionsPerSec
> for fetch requests?
>
> 2. TemporaryMessageSize for Produce/Fetch would indicate the space
> allocated for conversions. For other requests, this metric will not be
> created (unless we find a request where the size is large and this
> information is useful).
>
> Thank you,
>
> Rajini
>
>
> On Tue, Sep 5, 2017 at 4:55 PM, Ismael Juma <is...@juma.me.uk> wrote:
>
> > Thanks Rajini, +1 (binding) from me. Just a few minor comments:
> >
> > 1. FetchDownConversionsPerSec should probably be MessageConversionsPerSec
> > with a request tag for consistency with MessageConversionsTimeMs. The
> text
> > in that paragraph should also be updated to talk about message
> conversions
> > instead of down conversions only.
> >
> > 2. What will TemporaryMemorySize represent for requests other than
> > `ProduceRequest`?
> >
> > Ismael
> >
> > On Mon, Sep 4, 2017 at 2:09 PM, Rajini Sivaram <ra...@gmail.com>
> > wrote:
> >
> > > All the suggestions on the discuss thread have been incorporated into
> the
> > > KIP. Please let me know if you have any more concerns or else can we
> > > proceed with voting for this KIP?
> > >
> > > Thank you,
> > >
> > > Rajini
> > >
> > > On Thu, Aug 24, 2017 at 6:50 PM, Rajini Sivaram <
> rajinisivaram@gmail.com
> > >
> > > wrote:
> > >
> > > > Hi all,
> > > >
> > > > I would like to start the vote on KIP-188 that adds additional
> metrics
> > to
> > > > support health checks for Kafka Ops. Details are here:
> > > >
> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > 188+-+Add+new+metrics+to+support+health+checks
> > > >
> > > > Thank you,
> > > >
> > > > Rajini
> > > >
> > > >
> > >
> >
>
Re: [VOTE] KIP-188 - Add new metrics to support health checks
Posted by Rajini Sivaram <ra...@gmail.com>.
Hi Ismael,
1. At the moment FetchDownConversionsPerSec is a topic metric while
MessageConversionTimeMs is a request metric which indicates Produce/Fetch
as a tag. Are you suggesting that we should convert
FetchDownConversionsPerSec to a request metric called MessageConversionsPerSec
for fetch requests?
2. TemporaryMessageSize for Produce/Fetch would indicate the space
allocated for conversions. For other requests, this metric will not be
created (unless we find a request where the size is large and this
information is useful).
Thank you,
Rajini
On Tue, Sep 5, 2017 at 4:55 PM, Ismael Juma <is...@juma.me.uk> wrote:
> Thanks Rajini, +1 (binding) from me. Just a few minor comments:
>
> 1. FetchDownConversionsPerSec should probably be MessageConversionsPerSec
> with a request tag for consistency with MessageConversionsTimeMs. The text
> in that paragraph should also be updated to talk about message conversions
> instead of down conversions only.
>
> 2. What will TemporaryMemorySize represent for requests other than
> `ProduceRequest`?
>
> Ismael
>
> On Mon, Sep 4, 2017 at 2:09 PM, Rajini Sivaram <ra...@gmail.com>
> wrote:
>
> > All the suggestions on the discuss thread have been incorporated into the
> > KIP. Please let me know if you have any more concerns or else can we
> > proceed with voting for this KIP?
> >
> > Thank you,
> >
> > Rajini
> >
> > On Thu, Aug 24, 2017 at 6:50 PM, Rajini Sivaram <rajinisivaram@gmail.com
> >
> > wrote:
> >
> > > Hi all,
> > >
> > > I would like to start the vote on KIP-188 that adds additional metrics
> to
> > > support health checks for Kafka Ops. Details are here:
> > >
> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > 188+-+Add+new+metrics+to+support+health+checks
> > >
> > > Thank you,
> > >
> > > Rajini
> > >
> > >
> >
>
Re: [VOTE] KIP-188 - Add new metrics to support health checks
Posted by Ismael Juma <is...@juma.me.uk>.
Thanks Rajini, +1 (binding) from me. Just a few minor comments:
1. FetchDownConversionsPerSec should probably be MessageConversionsPerSec
with a request tag for consistency with MessageConversionsTimeMs. The text
in that paragraph should also be updated to talk about message conversions
instead of down conversions only.
2. What will TemporaryMemorySize represent for requests other than
`ProduceRequest`?
Ismael
On Mon, Sep 4, 2017 at 2:09 PM, Rajini Sivaram <ra...@gmail.com>
wrote:
> All the suggestions on the discuss thread have been incorporated into the
> KIP. Please let me know if you have any more concerns or else can we
> proceed with voting for this KIP?
>
> Thank you,
>
> Rajini
>
> On Thu, Aug 24, 2017 at 6:50 PM, Rajini Sivaram <ra...@gmail.com>
> wrote:
>
> > Hi all,
> >
> > I would like to start the vote on KIP-188 that adds additional metrics to
> > support health checks for Kafka Ops. Details are here:
> >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > 188+-+Add+new+metrics+to+support+health+checks
> >
> > Thank you,
> >
> > Rajini
> >
> >
>
Re: [VOTE] KIP-188 - Add new metrics to support health checks
Posted by Thomas Crayford <tc...@salesforce.com>.
+1 (non-binding)
On Mon, Sep 4, 2017 at 3:30 PM, Manikumar <ma...@gmail.com> wrote:
> +1 (non-binding)
>
> On Mon, Sep 4, 2017 at 6:39 PM, Rajini Sivaram <ra...@gmail.com>
> wrote:
>
> > All the suggestions on the discuss thread have been incorporated into the
> > KIP. Please let me know if you have any more concerns or else can we
> > proceed with voting for this KIP?
> >
> > Thank you,
> >
> > Rajini
> >
> > On Thu, Aug 24, 2017 at 6:50 PM, Rajini Sivaram <rajinisivaram@gmail.com
> >
> > wrote:
> >
> > > Hi all,
> > >
> > > I would like to start the vote on KIP-188 that adds additional metrics
> to
> > > support health checks for Kafka Ops. Details are here:
> > >
> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > 188+-+Add+new+metrics+to+support+health+checks
> > >
> > > Thank you,
> > >
> > > Rajini
> > >
> > >
> >
>
Re: [VOTE] KIP-188 - Add new metrics to support health checks
Posted by Manikumar <ma...@gmail.com>.
+1 (non-binding)
On Mon, Sep 4, 2017 at 6:39 PM, Rajini Sivaram <ra...@gmail.com>
wrote:
> All the suggestions on the discuss thread have been incorporated into the
> KIP. Please let me know if you have any more concerns or else can we
> proceed with voting for this KIP?
>
> Thank you,
>
> Rajini
>
> On Thu, Aug 24, 2017 at 6:50 PM, Rajini Sivaram <ra...@gmail.com>
> wrote:
>
> > Hi all,
> >
> > I would like to start the vote on KIP-188 that adds additional metrics to
> > support health checks for Kafka Ops. Details are here:
> >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > 188+-+Add+new+metrics+to+support+health+checks
> >
> > Thank you,
> >
> > Rajini
> >
> >
>
Re: [VOTE] KIP-188 - Add new metrics to support health checks
Posted by Rajini Sivaram <ra...@gmail.com>.
All the suggestions on the discuss thread have been incorporated into the
KIP. Please let me know if you have any more concerns or else can we
proceed with voting for this KIP?
Thank you,
Rajini
On Thu, Aug 24, 2017 at 6:50 PM, Rajini Sivaram <ra...@gmail.com>
wrote:
> Hi all,
>
> I would like to start the vote on KIP-188 that adds additional metrics to
> support health checks for Kafka Ops. Details are here:
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 188+-+Add+new+metrics+to+support+health+checks
>
> Thank you,
>
> Rajini
>
>
Re: [VOTE] KIP-188 - Add new metrics to support health checks
Posted by Rajini Sivaram <ra...@gmail.com>.
This vote has passed with 4 binding votes (Ismael, Jason, Jun, me) and 3
non-binding votes (Manikumar, Thomas Crayford, Mickael). Many thanks for
the votes and feedback.
I will update the KIP page.
Regards,
Rajini
On Fri, Sep 8, 2017 at 5:12 PM, Rajini Sivaram <ra...@gmail.com>
wrote:
> Thank you, Jun.
>
> If there are no other concerns by the end of the day, I will close the
> vote tomorrow.
>
>
> On Fri, Sep 8, 2017 at 5:09 PM, Jun Rao <ju...@confluent.io> wrote:
>
>> Hi, Raijini,
>>
>> Thanks for the explanation. We can leave those as they are then.
>>
>> Jun
>>
>> On Thu, Sep 7, 2017 at 3:23 AM, Rajini Sivaram <ra...@gmail.com>
>> wrote:
>>
>> > Hi all,
>> >
>> > I have added one more metric to KIP-188 to show the current status of
>> > broker's ZooKeeper connections. Please let me know if you have any
>> > concerns.
>> >
>> > Hi Jun,
>> >
>> > I was wondering which is a better group for
>> FetchMessageConversionsPerSec,
>> > now that we have MessageConversionsTimeMs at the request level. I don't
>> > have a strong opinion either way, but at the moment, as a topic metric,
>> > xxxMessageConversionsPerSec is along with MessagesInPerSec,
>> > TotalFetchRequestsPerSec etc. which are all kind of related and are all
>> > rate metrics. If we move it to the request level, we will
>> > have MessageConversionsTimeMs and MessageConversionsPerSec together, but
>> > will lose the topic grouping. Since all the other metrics at request
>> level
>> > are time histograms, perhaps it is better to leave
>> MessageConversionsPerSec
>> > along with the other topic rate metrics?
>> >
>> > I had added a ZooKeeperClient wrapper for my initial PR, but I will
>> rebase
>> > on Onur's code when it is ready. Thank you!
>> >
>> > Many thanks,
>> >
>> > Rajini
>> >
>> > On Wed, Sep 6, 2017 at 10:52 PM, Jun Rao <ju...@confluent.io> wrote:
>> >
>> > > Hi, Raijini,
>> > >
>> > > Thanks for the KIP. +1. Just a minor comment.
>> > >
>> > > Since we only measure MessageConversionsTimeMs at the request type
>> level,
>> > > is it useful to collect the following metrics at the topic level?
>> > >
>> > > *MBean*:
>> > > kafka.server:type=BrokerTopicMetrics,name=FetchMessageConver
>> sionsPerSec,
>> > > topic=([-.\w]+)
>> > >
>> > > *MBean*:
>> > > kafka.server:type=BrokerTopicMetrics,name=ProduceMessageConv
>> ersionsPerSe
>> > > c,topic=([-.\w]+)
>> > >
>> > >
>> > > Also, for the ZK latency metric, Onur added a new ZookeeperClient
>> wrapper
>> > > and is in the middle of converting existing zkClient usage to the new
>> > > wrapper. So, we probably want to add the latency metric in the new
>> > wrapper.
>> > >
>> > > Jun
>> > >
>> > > On Thu, Aug 24, 2017 at 10:50 AM, Rajini Sivaram <
>> > rajinisivaram@gmail.com>
>> > > wrote:
>> > >
>> > > > Hi all,
>> > > >
>> > > > I would like to start the vote on KIP-188 that adds additional
>> metrics
>> > to
>> > > > support health checks for Kafka Ops. Details are here:
>> > > >
>> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>> > > > 188+-+Add+new+metrics+to+support+health+checks
>> > > >
>> > > > Thank you,
>> > > >
>> > > > Rajini
>> > > >
>> > >
>> >
>>
>
>
Re: [VOTE] KIP-188 - Add new metrics to support health checks
Posted by Rajini Sivaram <ra...@gmail.com>.
Thank you, Jun.
If there are no other concerns by the end of the day, I will close the vote
tomorrow.
On Fri, Sep 8, 2017 at 5:09 PM, Jun Rao <ju...@confluent.io> wrote:
> Hi, Raijini,
>
> Thanks for the explanation. We can leave those as they are then.
>
> Jun
>
> On Thu, Sep 7, 2017 at 3:23 AM, Rajini Sivaram <ra...@gmail.com>
> wrote:
>
> > Hi all,
> >
> > I have added one more metric to KIP-188 to show the current status of
> > broker's ZooKeeper connections. Please let me know if you have any
> > concerns.
> >
> > Hi Jun,
> >
> > I was wondering which is a better group for
> FetchMessageConversionsPerSec,
> > now that we have MessageConversionsTimeMs at the request level. I don't
> > have a strong opinion either way, but at the moment, as a topic metric,
> > xxxMessageConversionsPerSec is along with MessagesInPerSec,
> > TotalFetchRequestsPerSec etc. which are all kind of related and are all
> > rate metrics. If we move it to the request level, we will
> > have MessageConversionsTimeMs and MessageConversionsPerSec together, but
> > will lose the topic grouping. Since all the other metrics at request
> level
> > are time histograms, perhaps it is better to leave
> MessageConversionsPerSec
> > along with the other topic rate metrics?
> >
> > I had added a ZooKeeperClient wrapper for my initial PR, but I will
> rebase
> > on Onur's code when it is ready. Thank you!
> >
> > Many thanks,
> >
> > Rajini
> >
> > On Wed, Sep 6, 2017 at 10:52 PM, Jun Rao <ju...@confluent.io> wrote:
> >
> > > Hi, Raijini,
> > >
> > > Thanks for the KIP. +1. Just a minor comment.
> > >
> > > Since we only measure MessageConversionsTimeMs at the request type
> level,
> > > is it useful to collect the following metrics at the topic level?
> > >
> > > *MBean*:
> > > kafka.server:type=BrokerTopicMetrics,name=
> FetchMessageConversionsPerSec,
> > > topic=([-.\w]+)
> > >
> > > *MBean*:
> > > kafka.server:type=BrokerTopicMetrics,name=
> ProduceMessageConversionsPerSe
> > > c,topic=([-.\w]+)
> > >
> > >
> > > Also, for the ZK latency metric, Onur added a new ZookeeperClient
> wrapper
> > > and is in the middle of converting existing zkClient usage to the new
> > > wrapper. So, we probably want to add the latency metric in the new
> > wrapper.
> > >
> > > Jun
> > >
> > > On Thu, Aug 24, 2017 at 10:50 AM, Rajini Sivaram <
> > rajinisivaram@gmail.com>
> > > wrote:
> > >
> > > > Hi all,
> > > >
> > > > I would like to start the vote on KIP-188 that adds additional
> metrics
> > to
> > > > support health checks for Kafka Ops. Details are here:
> > > >
> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > 188+-+Add+new+metrics+to+support+health+checks
> > > >
> > > > Thank you,
> > > >
> > > > Rajini
> > > >
> > >
> >
>
Re: [VOTE] KIP-188 - Add new metrics to support health checks
Posted by Jun Rao <ju...@confluent.io>.
Hi, Raijini,
Thanks for the explanation. We can leave those as they are then.
Jun
On Thu, Sep 7, 2017 at 3:23 AM, Rajini Sivaram <ra...@gmail.com>
wrote:
> Hi all,
>
> I have added one more metric to KIP-188 to show the current status of
> broker's ZooKeeper connections. Please let me know if you have any
> concerns.
>
> Hi Jun,
>
> I was wondering which is a better group for FetchMessageConversionsPerSec,
> now that we have MessageConversionsTimeMs at the request level. I don't
> have a strong opinion either way, but at the moment, as a topic metric,
> xxxMessageConversionsPerSec is along with MessagesInPerSec,
> TotalFetchRequestsPerSec etc. which are all kind of related and are all
> rate metrics. If we move it to the request level, we will
> have MessageConversionsTimeMs and MessageConversionsPerSec together, but
> will lose the topic grouping. Since all the other metrics at request level
> are time histograms, perhaps it is better to leave MessageConversionsPerSec
> along with the other topic rate metrics?
>
> I had added a ZooKeeperClient wrapper for my initial PR, but I will rebase
> on Onur's code when it is ready. Thank you!
>
> Many thanks,
>
> Rajini
>
> On Wed, Sep 6, 2017 at 10:52 PM, Jun Rao <ju...@confluent.io> wrote:
>
> > Hi, Raijini,
> >
> > Thanks for the KIP. +1. Just a minor comment.
> >
> > Since we only measure MessageConversionsTimeMs at the request type level,
> > is it useful to collect the following metrics at the topic level?
> >
> > *MBean*:
> > kafka.server:type=BrokerTopicMetrics,name=FetchMessageConversionsPerSec,
> > topic=([-.\w]+)
> >
> > *MBean*:
> > kafka.server:type=BrokerTopicMetrics,name=ProduceMessageConversionsPerSe
> > c,topic=([-.\w]+)
> >
> >
> > Also, for the ZK latency metric, Onur added a new ZookeeperClient wrapper
> > and is in the middle of converting existing zkClient usage to the new
> > wrapper. So, we probably want to add the latency metric in the new
> wrapper.
> >
> > Jun
> >
> > On Thu, Aug 24, 2017 at 10:50 AM, Rajini Sivaram <
> rajinisivaram@gmail.com>
> > wrote:
> >
> > > Hi all,
> > >
> > > I would like to start the vote on KIP-188 that adds additional metrics
> to
> > > support health checks for Kafka Ops. Details are here:
> > >
> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > 188+-+Add+new+metrics+to+support+health+checks
> > >
> > > Thank you,
> > >
> > > Rajini
> > >
> >
>
Re: [VOTE] KIP-188 - Add new metrics to support health checks
Posted by Rajini Sivaram <ra...@gmail.com>.
Hi all,
I have added one more metric to KIP-188 to show the current status of
broker's ZooKeeper connections. Please let me know if you have any concerns.
Hi Jun,
I was wondering which is a better group for FetchMessageConversionsPerSec,
now that we have MessageConversionsTimeMs at the request level. I don't
have a strong opinion either way, but at the moment, as a topic metric,
xxxMessageConversionsPerSec is along with MessagesInPerSec,
TotalFetchRequestsPerSec etc. which are all kind of related and are all
rate metrics. If we move it to the request level, we will
have MessageConversionsTimeMs and MessageConversionsPerSec together, but
will lose the topic grouping. Since all the other metrics at request level
are time histograms, perhaps it is better to leave MessageConversionsPerSec
along with the other topic rate metrics?
I had added a ZooKeeperClient wrapper for my initial PR, but I will rebase
on Onur's code when it is ready. Thank you!
Many thanks,
Rajini
On Wed, Sep 6, 2017 at 10:52 PM, Jun Rao <ju...@confluent.io> wrote:
> Hi, Raijini,
>
> Thanks for the KIP. +1. Just a minor comment.
>
> Since we only measure MessageConversionsTimeMs at the request type level,
> is it useful to collect the following metrics at the topic level?
>
> *MBean*:
> kafka.server:type=BrokerTopicMetrics,name=FetchMessageConversionsPerSec,
> topic=([-.\w]+)
>
> *MBean*:
> kafka.server:type=BrokerTopicMetrics,name=ProduceMessageConversionsPerSe
> c,topic=([-.\w]+)
>
>
> Also, for the ZK latency metric, Onur added a new ZookeeperClient wrapper
> and is in the middle of converting existing zkClient usage to the new
> wrapper. So, we probably want to add the latency metric in the new wrapper.
>
> Jun
>
> On Thu, Aug 24, 2017 at 10:50 AM, Rajini Sivaram <ra...@gmail.com>
> wrote:
>
> > Hi all,
> >
> > I would like to start the vote on KIP-188 that adds additional metrics to
> > support health checks for Kafka Ops. Details are here:
> >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > 188+-+Add+new+metrics+to+support+health+checks
> >
> > Thank you,
> >
> > Rajini
> >
>
Re: [VOTE] KIP-188 - Add new metrics to support health checks
Posted by Jun Rao <ju...@confluent.io>.
Hi, Raijini,
Thanks for the KIP. +1. Just a minor comment.
Since we only measure MessageConversionsTimeMs at the request type level,
is it useful to collect the following metrics at the topic level?
*MBean*:
kafka.server:type=BrokerTopicMetrics,name=FetchMessageConversionsPerSec,topic=([-.\w]+)
*MBean*:
kafka.server:type=BrokerTopicMetrics,name=ProduceMessageConversionsPerSec,topic=([-.\w]+)
Also, for the ZK latency metric, Onur added a new ZookeeperClient wrapper
and is in the middle of converting existing zkClient usage to the new
wrapper. So, we probably want to add the latency metric in the new wrapper.
Jun
On Thu, Aug 24, 2017 at 10:50 AM, Rajini Sivaram <ra...@gmail.com>
wrote:
> Hi all,
>
> I would like to start the vote on KIP-188 that adds additional metrics to
> support health checks for Kafka Ops. Details are here:
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 188+-+Add+new+metrics+to+support+health+checks
>
> Thank you,
>
> Rajini
>