[GitHub] [kafka] divijvaidya commented on pull request #13798: KAFKA-15028: AddPartitionsToTxnManager metrics

2023-06-28 Thread via GitHub


divijvaidya commented on PR #13798:
URL: https://github.com/apache/kafka/pull/13798#issuecomment-1611605733

   > so only that one verification per transaction
   
   ah, I had missed this part that it will not be recorded on "every" message 
append. Only for verified cases. I think we should be good to merge this in 
without worrying about the latency impact. I don't suppose a single histogram 
should add much and we also didn't see an impact in the producer perf test that 
you did here.


-- 
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-unsubscr...@kafka.apache.org

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



[GitHub] [kafka] divijvaidya commented on pull request #13798: KAFKA-15028: AddPartitionsToTxnManager metrics

2023-06-27 Thread via GitHub


divijvaidya commented on PR #13798:
URL: https://github.com/apache/kafka/pull/13798#issuecomment-1609922486

   > if this does affect performance too much
   
   Maybe we could use some type of recoding level for Yammer metrics too? We 
already have a configuration at:
   
https://kafka.apache.org/documentation.html#brokerconfigs_metrics.recording.level.
 We can emit this metric at DEBUG level.
   
   Also, FYI, here [1] is the flamegraph where you can observe the impact of 
histogram.update() for requests. After opening this flamegraph in browser, you 
can look at call stack of processCompletedSends
   
   [1] 
https://github.com/divijvaidya/flamegraph-samples/blob/main/kafka/kafka-summit-2023/100mslinger-cpu-unclean.html
 


-- 
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-unsubscr...@kafka.apache.org

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



[GitHub] [kafka] divijvaidya commented on pull request #13798: KAFKA-15028: AddPartitionsToTxnManager metrics

2023-06-27 Thread via GitHub


divijvaidya commented on PR #13798:
URL: https://github.com/apache/kafka/pull/13798#issuecomment-1609327501

   Another point I want to call out is that Yammer metrics histogram is 
notorious for consuming CPU (and increase latency). It consumes ~4-5% CPU on 
the network threads for calculating per request latency. Given that we are 
doing this on the latency critical append path (right?), I suspect we may 
encounter some latency increase + CPU increase. Although one histogram 
calculation here should be ok, but it would be nice if you get some 
producer-perf.sh data in as well to ensure that this metric isn't adversely 
impacting latency. Thoughts?


-- 
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-unsubscr...@kafka.apache.org

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