Updated metric description as well. -Artem
On Fri, Jul 8, 2022 at 9:22 AM Jun Rao <j...@confluent.io.invalid> wrote: > 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 > > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
