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 <ism...@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 <rajinisiva...@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 <ism...@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 < > > rajinisiva...@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 < > > > rajinisiva...@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 > > > > > > > > > > > > > > > > > > > > > > > > > > >
