Hi, Artem, Thanks for the reply. It would be useful to add that clarification in the description of the metric. Other than that, the KIP looks good to me.
Jun On Tue, Jul 5, 2022 at 5:57 PM Artem Livshits <alivsh...@confluent.io.invalid> wrote: > I've updated the KIP to clarify that the metric reflects the total amount > of producer ids in all partitions maintained in the broker. > > -Artem > > On Thu, Jun 30, 2022 at 11:46 AM Jun Rao <j...@confluent.io.invalid> wrote: > > > Hi, Artem, > > > > Thanks for the reply. > > > > The memory usage on the broker is proportional to the number of > (partition, > > pid) combinations. So, I am wondering if we could have a metric that > > captures that. The proposed pid count metric doesn't fully capture that > > since each pid could be associated with a different number of partitions. > > > > Thanks, > > > > Jun > > > > On Thu, Jun 30, 2022 at 9:24 AM Justine Olshan > > <jols...@confluent.io.invalid> > > wrote: > > > > > Hi Artem, > > > Thanks for the update to include motivation. Makes sense to me. > > > Justine > > > > > > On Wed, Jun 29, 2022 at 6:51 PM Luke Chen <show...@gmail.com> wrote: > > > > > > > Hi Artem, > > > > > > > > Thanks for the update. > > > > LGTM. > > > > > > > > Luke > > > > > > > > On Thu, Jun 30, 2022 at 6:51 AM Artem Livshits > > > > <alivsh...@confluent.io.invalid> wrote: > > > > > > > > > Thank you for your feedback. I've updated the KIP to elaborate on > the > > > > > motivation and provide some background on producer ids and how we > > > measure > > > > > them. > > > > > > > > > > Also, after some thinking and discussing it offline with some > folks, > > I > > > > > think that we don't really need partitioner level metrics. We can > > use > > > > > existing tools to do granular debugging. I've moved partition > level > > > > > metrics to the rejected alternatives section. > > > > > > > > > > -Artem > > > > > > > > > > On Wed, Jun 29, 2022 at 1:57 AM Luke Chen <show...@gmail.com> > wrote: > > > > > > > > > > > Hi Artem, > > > > > > > > > > > > Could you elaborate more in the motivation section? > > > > > > I'm interested to know what kind of scenarios this metric can > > benefit > > > > > for. > > > > > > What could it bring to us when a topic partition has 100 > > > > ProducerIdCount > > > > > VS > > > > > > another topic partition has 10 ProducerIdCount? > > > > > > > > > > > > Thank you. > > > > > > Luke > > > > > > > > > > > > On Wed, Jun 29, 2022 at 6:30 AM Jun Rao <j...@confluent.io.invalid > > > > > > > wrote: > > > > > > > > > > > > > Hi, Artem, > > > > > > > > > > > > > > Thanks for the KIP. > > > > > > > > > > > > > > Could you explain the partition level ProducerIdCount a bit > more? > > > > Does > > > > > > that > > > > > > > reflect the number of PIDs ever produced to a partition since > the > > > > > broker > > > > > > is > > > > > > > started? Do we reduce the count after a PID expires? > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > Jun > > > > > > > > > > > > > > On Wed, Jun 22, 2022 at 1:08 AM David Jacot > > > > > <dja...@confluent.io.invalid > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > Hi Artem, > > > > > > > > > > > > > > > > The KIP LGTM. > > > > > > > > > > > > > > > > Thanks, > > > > > > > > David > > > > > > > > > > > > > > > > On Tue, Jun 21, 2022 at 9:32 PM Artem Livshits > > > > > > > > <alivsh...@confluent.io.invalid> wrote: > > > > > > > > > > > > > > > > > > If there is no other feedback I'm going to start voting in > a > > > > couple > > > > > > > days. > > > > > > > > > > > > > > > > > > -Artem > > > > > > > > > > > > > > > > > > On Fri, Jun 17, 2022 at 3:50 PM Artem Livshits < > > > > > > alivsh...@confluent.io > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > Thank you for your feedback. Updated the KIP and added > the > > > > > > Rejected > > > > > > > > > > Alternatives section. > > > > > > > > > > > > > > > > > > > > -Artem > > > > > > > > > > > > > > > > > > > > On Fri, Jun 17, 2022 at 1:16 PM Ismael Juma < > > > ism...@juma.me.uk > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > >> If we don't track them separately, then it makes sense > to > > > keep > > > > > it > > > > > > as > > > > > > > > one > > > > > > > > > >> metric. I'd probably name it ProducerIdCount in that > case. > > > > > > > > > >> > > > > > > > > > >> Ismael > > > > > > > > > >> > > > > > > > > > >> On Fri, Jun 17, 2022 at 12:04 PM Artem Livshits > > > > > > > > > >> <alivsh...@confluent.io.invalid> wrote: > > > > > > > > > >> > > > > > > > > > >> > Do you propose to have 2 metrics instead of one? > Right > > > now > > > > we > > > > > > > don't > > > > > > > > > >> track > > > > > > > > > >> > if the producer id was transactional or idempotent and > > for > > > > > > metric > > > > > > > > > >> > collection we'd either have to pay the cost of > iterating > > > > over > > > > > > > > producer > > > > > > > > > >> ids > > > > > > > > > >> > (which could be a lot) or split the producer map into > 2 > > or > > > > > cache > > > > > > > the > > > > > > > > > >> > counts, which complicates the code. > > > > > > > > > >> > > > > > > > > > > >> > From the monitoring perspective, I think one metric > > should > > > > be > > > > > > > good, > > > > > > > > but > > > > > > > > > >> > maybe I'm missing some scenarios. > > > > > > > > > >> > > > > > > > > > > >> > -Artem > > > > > > > > > >> > > > > > > > > > > >> > On Fri, Jun 17, 2022 at 12:28 AM Ismael Juma < > > > > > ism...@juma.me.uk > > > > > > > > > > > > > > > wrote: > > > > > > > > > >> > > > > > > > > > > >> > > I like the suggestion to have > IdempotentProducerCount > > > and > > > > > > > > > >> > > TransactionalProducerCount metrics. > > > > > > > > > >> > > > > > > > > > > > >> > > Ismael > > > > > > > > > >> > > > > > > > > > > > >> > > On Thu, Jun 16, 2022 at 2:27 PM Artem Livshits > > > > > > > > > >> > > <alivsh...@confluent.io.invalid> wrote: > > > > > > > > > >> > > > > > > > > > > > >> > > > Hi Ismael, > > > > > > > > > >> > > > > > > > > > > > > >> > > > Thank you for your feedback. Yes, this is > counting > > > the > > > > > > number > > > > > > > > of > > > > > > > > > >> > > producer > > > > > > > > > >> > > > ids tracked by the partition and broker. Another > > > > options > > > > > I > > > > > > > was > > > > > > > > > >> > thinking > > > > > > > > > >> > > of > > > > > > > > > >> > > > are the following: > > > > > > > > > >> > > > > > > > > > > > > >> > > > - IdempotentProducerCount > > > > > > > > > >> > > > - TransactionalProducerCount > > > > > > > > > >> > > > - ProducerIdCount > > > > > > > > > >> > > > > > > > > > > > > >> > > > Let me know if one of these seems better, or I'm > > open > > > to > > > > > > other > > > > > > > > name > > > > > > > > > >> > > > suggestions as well. > > > > > > > > > >> > > > > > > > > > > > > >> > > > -Artem > > > > > > > > > >> > > > > > > > > > > > > >> > > > On Wed, Jun 15, 2022 at 11:49 PM Ismael Juma < > > > > > > > ism...@juma.me.uk > > > > > > > > > > > > > > > > > > >> > wrote: > > > > > > > > > >> > > > > > > > > > > > > >> > > > > Thanks for the KIP. > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > ProducerCount seems like a misleading name since > > > > > producers > > > > > > > > > >> without a > > > > > > > > > >> > > > > producer id are not counted. Is this meant to > > count > > > > the > > > > > > > > number of > > > > > > > > > >> > > > producer > > > > > > > > > >> > > > > IDs tracked by the broker? > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > Ismael > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > On Wed, Jun 15, 2022, 3:12 PM Artem Livshits < > > > > > > > > > >> alivsh...@confluent.io > > > > > > > > > >> > > > > .invalid> > > > > > > > > > >> > > > > wrote: > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > Hello, > > > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > I'd like to start a discussion on the KIP-847: > > > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-847%3A+Add+ProducerCount+metrics > > > > > > > > > >> > > > > > . > > > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > -Artem > > > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
