You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2023/01/03 14:21:45 UTC

[GitHub] [kafka] clolov opened a new pull request, #13068: KAFKA-13999: Add ProducerIdCount metric

clolov opened a new pull request, #13068:
URL: https://github.com/apache/kafka/pull/13068

   _Summary_
   This PR provides an implementation of KIP-847 (https://cwiki.apache.org/confluence/display/KAFKA/KIP-847%3A+Add+ProducerIdCount+metrics). As far as I can tell, the original author @artemlivshits has not provided a pull request since the KIP was approved so I took the liberty of picking it up. Of course, if they provide a pull request I am happy to close this one.
   
   I do not understand why the metric ought to be surfaced from `kafka.server:type=ReplicaManager,name=ProducerIdCount` when the information is stored in `ProducerStateManager`. I have thus taken the further liberty of surfacing the metric from `ProducerStateManager` directly under `kafka.log:type=ProducerStateManager,name=ProducerIdCount`. I am happy to revise this given the feedback or if this is accepted to update the KIP.
   
   _Testing_
   Grafana dashboard showing ProducerIdCount increasing when adding idempotent producers and decreasing whenever the the producer ids expire: https://g-576b9cd7b5.grafana-workspace.us-east-1.amazonaws.com/dashboard/snapshot/U5NfIDaMlm0VTIvmF4ZHJio7Kwuc1pTk
   
   I do not know whether we carry out any unit/integration testing upon addition of new metrics, but if I am pointed to similar test cases I am happy to revise this PR.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] clolov closed pull request #13068: KAFKA-13999: Add ProducerIdCount metric

Posted by GitBox <gi...@apache.org>.
clolov closed pull request #13068: KAFKA-13999: Add ProducerIdCount metric
URL: https://github.com/apache/kafka/pull/13068


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] clolov commented on pull request #13068: KAFKA-13999: Add ProducerIdCount metric

Posted by GitBox <gi...@apache.org>.
clolov commented on PR #13068:
URL: https://github.com/apache/kafka/pull/13068#issuecomment-1369826380

   For visibility:
   @artemlivshits - as the original author
   @showuon - as you commented on the JIRA ticket and I believe you have the knowledge to review the change
   @jolshan - as you worked on https://issues.apache.org/jira/browse/KAFKA-14097 and I believe you also have the background knowledge needed to review this change 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] jolshan commented on pull request #13068: KAFKA-13999: Add ProducerIdCount metric

Posted by GitBox <gi...@apache.org>.
jolshan commented on PR #13068:
URL: https://github.com/apache/kafka/pull/13068#issuecomment-1371625876

   Here's the alternative PR: https://github.com/apache/kafka/pull/13078 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] jolshan commented on pull request #13068: KAFKA-13999: Add ProducerIdCount metric

Posted by GitBox <gi...@apache.org>.
jolshan commented on PR #13068:
URL: https://github.com/apache/kafka/pull/13068#issuecomment-1370083728

   Ah I believe had a slightly different idea for implementation where the size was not evaluated on every call, but rather a variable was updated every time we added or removed producer IDs. Let me see if he can share his version.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] jolshan commented on a diff in pull request #13068: KAFKA-13999: Add ProducerIdCount metric

Posted by GitBox <gi...@apache.org>.
jolshan commented on code in PR #13068:
URL: https://github.com/apache/kafka/pull/13068#discussion_r1062016540


##########
core/src/main/scala/kafka/log/ProducerStateManager.scala:
##########
@@ -502,6 +503,8 @@ class ProducerStateManager(
   private var lastMapOffset = 0L
   private var lastSnapOffset = 0L
 
+  newGauge("ProducerIdCount", () => producers.size)

Review Comment:
   I think the other concern with this approach is that we have this metric per ProducerStateManager (per partition!) when we really want it per broker. https://github.com/apache/kafka/pull/13078 resolves this.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] clolov commented on pull request #13068: KAFKA-13999: Add ProducerIdCount metric

Posted by GitBox <gi...@apache.org>.
clolov commented on PR #13068:
URL: https://github.com/apache/kafka/pull/13068#issuecomment-1374723793

   Got it, thank you, I am closing this and moving my comments to the other pull request.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org