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 > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
