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/09/04 13:09:43 UTC
Re: [VOTE] KIP-188 - Add new metrics to support health checks
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 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
> >
> >
>